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(transaction): Fix auto journaling in transaction #4737

Conversation

BagritsevichStepan
Copy link
Contributor

@BagritsevichStepan BagritsevichStepan commented Mar 10, 2025

  1. The first bug is that we were not resetting re_enabled_auto_journal_ in the transaction. For example, if we are in multi-mode and one of the commands calls ReviveAutoJournal, the next commands with CO::NO_AUTOJOURNAL will still be auto-journaled because we didn't set re_enabled_auto_journal_ to false
  2. The second bug is specific to the STREAM logic. The streamId is generated using Unix time, which can differ between the master and the replica, potentially causing replication issues. To fix this, we need to pass time_now_ms from the master to the replica. To achieve this, the TIMENOW option was added to the XADD command.

@BagritsevichStepan BagritsevichStepan self-assigned this Mar 10, 2025
@BagritsevichStepan BagritsevichStepan force-pushed the replication/fix-auto-journaling branch from 851441e to dc1ef4a Compare March 10, 2025 12:14
Signed-off-by: Stepan Bagritsevich <stefan@dragonflydb.io>
@BagritsevichStepan BagritsevichStepan force-pushed the replication/fix-auto-journaling branch from dc1ef4a to 5d42c56 Compare March 10, 2025 12:15
Signed-off-by: Stepan Bagritsevich <stefan@dragonflydb.io>
@BagritsevichStepan BagritsevichStepan enabled auto-merge (squash) March 10, 2025 15:44
@BagritsevichStepan BagritsevichStepan merged commit 4f70d1b into dragonflydb:main Mar 10, 2025
10 checks passed
@BagritsevichStepan BagritsevichStepan deleted the replication/fix-auto-journaling branch March 10, 2025 16:27
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.

3 participants