incrementalFetch(): fix race condition which would miss rows based on changed_at#288
incrementalFetch(): fix race condition which would miss rows based on changed_at#288
Conversation
|
Before you spend any more time looking into this, please take a look at #237. |
Don't worry. I was already done. (#288 (comment), #289)
Seems more complicated to me. 😅 |
|
Please rebase to rerun the failing CI job, fixed in the current main branch. |
|
This is just a workaround. I prefer to fix the root cause properly, i.e: If you agree, please review that PR's intention (description) itself, not necessarily the PHP code. |
… changed_at incrementalFetch() relies on every newly inserted row having a higher changed_at than every already committed one. At the moment, web doesn't guarantee this. It even writes changed_at rounded to whole seconds. Hence, we could skip one of two rows which appeared during the same second. This change simply ignores the most recent 3s of config updates (for 3s), so that two or more new rows which appear during the same second aren't a problem. They're processed together 3s after they get committed.
Al2Klimov
force-pushed
the
changed_at-racecond
branch
from
ab1d223 to
d5110bb
Compare
|
|
@nilmerg You said you don't like voluntary thresholds like these 3s. But Icinga DB already gives up after the DB is down for 5m which is also a voluntary threshold. :) |
|
As discussed offline with @nilmerg, I think a changed_at map per row id, and not just per table, could also solve this, but this PR is the way of the least resistance. |
|
Can you please stop doing random things. Yes, I'm aware that there are limitations with the current implementation, but now doing things without any coordination between @nilmerg and me for such core mechanism makes little sense. And now this ist getting quite messy here: |
|
3 participants
incrementalFetch() relies on every newly inserted row having a higher changed_at than every already committed one. At the moment, web doesn't guarantee this. It even writes changed_at rounded to whole seconds. Hence, we could skip one of two rows which appeared during the same second.
This change simply ignores the most recent 3s of config updates (for 3s), so that two or more new rows which appear during the same second aren't a problem. They're processed together 3s after they get committed.
closes #237