-
Notifications
You must be signed in to change notification settings - Fork 214
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
State cb #545
State cb #545
Conversation
Fixes: 1. Tests were no longer able to test the condition where WAL frames were being applied while a tx was open, because the test didn't run long enough. 2. A race in the tests where some systems generate a barrier while a barrier was active, making it difficult to wait for the right log entry. Signed-off-by: Mathieu Borderé <mathieu.bordere@canonical.com>
The state_cb is triggered immediately when the raft state changes, instead of the monitor_cb, that is only polling for the state change at the start of a loop iteration. This avoids some bugs where dqlite has to perform cleanups on leadership loss, but wasn't informed yet of the leadership loss. Signed-off-by: Mathieu Borderé <mathieu.bordere@canonical.com>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #545 +/- ##
==========================================
- Coverage 61.37% 61.29% -0.08%
==========================================
Files 34 34
Lines 6842 6834 -8
Branches 2031 2031
==========================================
- Hits 4199 4189 -10
Misses 1349 1349
- Partials 1294 1296 +2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, could you elaborate on why the test changes are needed?
The unchanged tests only waited for the leader barrier to be applied, so they didn't test an ex-leader trying to apply frames to it's WAL. The changes make the test explicitly wait for the barrier first and then wait for the This test wasn't adapted after the raft "barrier on become leader" change was introduced because they still passed. Probably there are more cases like this ... |
The state_cb is triggered immediately when the raft state changes,
instead of the monitor_cb, that is only polling for the state change at
the start of a loop iteration. This avoids some bugs where dqlite has to
perform cleanups on leadership loss, but wasn't informed yet of the
leadership loss.
fixes #541
Note: When we release a new dqlite version, we will also need an
accompanying new raft version that supports registering the state_cb.