Skip to content
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

Ensure that the index processed by the client is at least as new as the last one processed. #18269

Commits on Aug 21, 2023

  1. Ensure that the index processed by the client is at least as new as t…

    …he last index processed so that stale data does not impact the running allocations.
    stswidwinski committed Aug 21, 2023
    Configuration menu
    Copy the full SHA
    220eb51 View commit details
    Browse the repository at this point in the history

Commits on Aug 22, 2023

  1. Add a smoke test for the issue.

    Testing for absence is a rather difficult task without hooks into the code to assert that given code paths have been hit. In this test we simply make sure that a transaction with a lower raft index does not make it to the client and depend on the timeout to make sure that this is the case. If the invariant is broken we expect the test to become either flaky or entirely broken.
    
    I acknowledge there may be a better way to test here, but lacking context this seems to be a minimal attempt to ensure that a similar issue doesn't creep back in. Feedback welcome.
    stswidwinski committed Aug 22, 2023
    Configuration menu
    Copy the full SHA
    d24f644 View commit details
    Browse the repository at this point in the history
  2. Add unlock.

    stswidwinski committed Aug 22, 2023
    Configuration menu
    Copy the full SHA
    0db169a View commit details
    Browse the repository at this point in the history
  3. Reverting the proposed tests for a few reasons:

    1. error is ignored in WaitForResult
    2. existing timeouts in blocking queries cannot be overridden and would make the tests always pass
    3. the scheduler seems to ignore raft transactions with lower ids
    
    As such, we would need more infrastructure to write such tests. Do you have an idea is there is a precedent I can use here no to reinvent the wheel? In general we need:
    
    1. To reduce the raft index of scheduler (purge the state or reassign the state: This can be done with regular assignment into a newly created state
    2. Induce a timeout on the blocking query to ensure that the query result is passed to client -> This I see no easy way of achieving
    stswidwinski committed Aug 22, 2023
    Configuration menu
    Copy the full SHA
    1b7b43c View commit details
    Browse the repository at this point in the history

Commits on Aug 25, 2023

  1. Update client/client.go

    Use identifier-style keys for structured logs
    
    Co-authored-by: Michael Schurter <michael.schurter@gmail.com>
    stswidwinski and schmichael committed Aug 25, 2023
    Configuration menu
    Copy the full SHA
    5d57896 View commit details
    Browse the repository at this point in the history
  2. add cl

    schmichael committed Aug 25, 2023
    Configuration menu
    Copy the full SHA
    83310de View commit details
    Browse the repository at this point in the history