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

fix: Incorrect commitId gets updated against commit entry when a batc… #1045

Merged
merged 1 commit into from
May 25, 2023

Conversation

sitaram-kalluri
Copy link
Member

- What I did

  • In the uncommitted entries list, when preparing the Sync batch request, if there exist two entries of the same key where the initial commit entry is "update" and later is "delete", then the commit entry with "update" operation is skipped adding to the Batch Request. In this case, the size of the batch request is one less than the size of uncommitted entries.

  • On receiving the batch response, the uncommitted list is iterated and the commit id from the batch response is updated to its respective commit entry. Since an entry is skipped in the batch request (but the entry exists in the uncommitted entries list), the commit id gets updated to the skipped entry (here, the entry with CommitOp. update). The commit-id of the entry synced to the server will remain null (here, the entry with CommitOp.Delete will not be updated).

- How I did it

  • In the "getBatchRequests" method, introduce a temporary list - "removeUncommittedEntriesList". When preparing the Sync batch request, if a key is skipped, then add it to "removeUncommittedEntriesList". In the end, iterate "removeUncommittedEntriesList" and remove the entries from the uncommitted entries list. Also, to avoid stale commit-entry in the commit log keystore, remove the entry from the keystore.

- How to verify it

  • Added a unit test that asserts the following:

    • In an uncommitted entries list, if the same key is updated and deleted, the commit id should be updated against deleted entry.
  • Added a functional test that asserts the same as above.

- Description for the changelog

  • Incorrect commit-Id gets updated against commit entry when a batch skips an entry

@gkc
Copy link
Contributor

gkc commented May 25, 2023

LGTM

@sitaram-kalluri sitaram-kalluri merged commit 8eb4868 into trunk May 25, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants