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.

Sign up for GitHub

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

Jump to bottom

Fix inconsistency after InstallSnapshot #128

Merged
merged 32 commits into from
Mar 15, 2022
Merged

Fix inconsistency after InstallSnapshot #128

merged 32 commits into from
Mar 15, 2022

Conversation

negokaz
Copy link
Contributor

@negokaz negokaz commented Feb 16, 2022 •
edited
Loading

Closes #127

Overview

  • Introduce new event SnapshotCopied that SnapshotSyncManager saves after copying snapshots
    • This event contains entityIds that indicate that the snapshot that was copied
    • Tags this event with EntitySnapshotsUpdatedTag(CompactionCompletedTag)
    • This tagging allows that follower's SnapshotSyncManager reads SnapshotCopieds that the leader's SnapshotSyncManager persisted
    • Follower members can know updated snapshots with SnapshotCopied that produced by snapshot synchronization that the leader has executed
  • SnapshotSyncManager reads CompactionCompleted and also SnapshotCopied
    • SnapshotSyncManager can identify all entityIds that should copy snapshots

NOTE

This change does not fix the potential consistency problem.
Systems that already have executed InstallSnapshot may still occur this issue even if applying this change.

Taichi Yamakawa and others added 19 commits February 15, 2022 08:27
Add a multi-jvm test: MultiSnapshotSyncSpec
e6c8014 
The snapshot synchronization should retrieve snapshots installed by another synchronization.
@negokaz
✨feat: Add SnapshotCopied event
bf44ff1 
This event indicates that the snapshots has been updated by snapshot synchronization.
The other member which receives InstallSnapshot from this member will read the event.
@negokaz
✨feat: Collects SnapshotCopied for snapshot synchronization
a34050f
@negokaz
✨feat: Record progress of snapshot synchronization with SnapshotCopied
5b85c79
@negokaz
🚨test: Add receivePersisted to RaftEventJournalTestKit
5d73cc9
@negokaz
🚨test: Add expectNothingPersisted to RaftEventJournalTestKit
8bb610d
@negokaz
🚨test: Add createUniqueEntityId
810f557
@negokaz
🚨test: Arrange existing test cases to use SnapshotCopied
7ee04d4
@negokaz
🚨test: SnapshotSyncManager should persist SnapshotCopied event every …
28ad682 
…copying snapshots batch
@negokaz
🔨refactor: Rename CompactionCompletedTag to EntitySnapshotsUpdatedTag
33ffa53
@negokaz
📚doc: RaftActor and SnapshotSyncManager have to use the same journal …
cf609b8 
…plugin
@negokaz
👷chore: Add akka-stream-testkit dependency in test scope
68d5a45
@negokaz
🚨test: Persisted SnapshotCopied is tagged
fc213da
@negokaz
🐞fix: Apply journalPluginAdditionalConfig to SnapshotSyncManager to e…
abf0209 
…nable RaftEventAdapter
@negokaz
🔨refactor: Check dstLatestSnapshotLastLogTerm and dstLatestSnapshotLa…
c658270 
…stLogIndex with 'filter'
@negokaz
🔨refactor: Rename CompactionEnvelope to EntitySnapshotsUpdated
9fd0bdf
@negokaz
🚀perf: Merge into single EntitySnapshotsUpdated before copying snapshots
0428608
@negokaz
🔀Merge remote-tracking branch 'origin/multi-snapshot-sync-spec'
6dd08ca
@negokaz
👷chore: Add mima-filter
7b6e8c7
@negokaz negokaz force-pushed the fix-inconsistency-after-installsnapshot branch from 748c664 to c1ed3d3 Compare February 16, 2022 04:42
@negokaz negokaz marked this pull request as ready for review February 16, 2022 04:43
@xirc xirc self-requested a review February 17, 2022 01:51
xirc
xirc reviewed Feb 17, 2022
View reviewed changes
Copy link
Contributor

@xirc xirc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great👍

I left some comments. I will review SnapshotSyncManagerSpec and RaftEventJournalTestKitSpec from now.

src/test/scala/lerna/akka/entityreplication/util/EventStore.scala Outdated Show resolved Hide resolved
src/test/scala/lerna/akka/entityreplication/util/RaftEventJournalTestKit.scala Outdated Show resolved Hide resolved
src/test/scala/lerna/akka/entityreplication/util/RaftEventJournalTestKit.scala Outdated Show resolved Hide resolved
src/main/scala/lerna/akka/entityreplication/raft/RaftSettingsImpl.scala Show resolved Hide resolved
src/main/scala/lerna/akka/entityreplication/raft/snapshot/sync/SnapshotSyncManager.scala Show resolved Hide resolved
src/test/scala/lerna/akka/entityreplication/util/RaftEventJournalTestKit.scala Outdated Show resolved Hide resolved
Taichi Yamakawa and others added 2 commits February 17, 2022 18:47
Fix election-timeout override in MultiSnapshotSyncSpec
e17e416 
The override path should be "lerna.akka.entityreplication.raft.election-timeout".
@negokaz
✨feat: Validate 'snapshot-sync.max-snapshot-batch-size'
71bf73a
xirc
xirc reviewed Feb 18, 2022
View reviewed changes
Copy link
Contributor

@xirc xirc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test code RaftEventJournalTestKitSpec and SnapshotSyncManagerSpec LGTM 👍

src/test/scala/lerna/akka/entityreplication/raft/snapshot/sync/SnapshotSyncManagerSpec.scala Outdated Show resolved Hide resolved
@negokaz negokaz force-pushed the fix-inconsistency-after-installsnapshot branch from 1f6df99 to ba4a77c Compare February 18, 2022 04:57
xirc
xirc previously approved these changes Feb 18, 2022
View reviewed changes
Copy link
Contributor

@xirc xirc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent! LGTM 👍

@xirc xirc enabled auto-merge February 18, 2022 07:14
xirc
xirc approved these changes Mar 15, 2022
View reviewed changes
Copy link
Contributor

@xirc xirc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@xirc xirc merged commit 71c1cef into master Mar 15, 2022
@xirc xirc deleted the fix-inconsistency-after-installsnapshot branch March 15, 2022 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xirc xirc xirc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

InstallSnapshot can cause inconsistency (Entity grasp is insufficient)
2 participants
@negokaz @xirc