-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Point in time snapshots #1471
base: master
Are you sure you want to change the base?
Point in time snapshots #1471
Conversation
StartedAt: time.Now(), | ||
|
||
store: store, | ||
exitCh: make(chan bool, 5), |
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.
For multiple store instances there'd be snapshot run triggered at one point of time, in that case why'd we need multiple exitCh
chan.
From my understanding flow is like below:
- Each Store has one snapshot
- Each snapshot has one flusher
- Each flusher sends exit signal after flush completed for all objects in that store
Also my assumption is each store will create it's own snapshotter and won't be shared among stores is that correct?
Please correct me if I'm wrong.
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.
Yes, the size of the exit channel can be 1 as well.
Yes, this is right. However, this PR will not cover the SnapshotManager
that will spawn a snapshotter process for each of the stores.
Also my assumption is each store will create it's own snapshotter and won't be shared among stores is that correct?
ID uint64 | ||
|
||
store SnapshotStore | ||
totalStoreShot uint8 |
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.
What are we using this variable for?
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.
I think this isn't required now, will remove it.
for { | ||
select { | ||
case <-pit.ctx.Done(): | ||
pit.Close() | ||
return | ||
case <-pit.exitCh: | ||
pit.Close() | ||
return | ||
} | ||
} | ||
return |
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.
nit: Shall we update this to?
func (pit *PointInTimeSnapshot) processStoreUpdates() error {
select {
case <-pit.ctx.Done():
case <-pit.exitCh:
}
pit.Close()
return nil
}
The PR implements snapshotting a single store into the disk using the copy-on-write approach.
Check the
README.md
in the PR for more details on the design and implementation.The PR doesn't implement the strategy of multi shard snapshots yet. It will be implemented
in a subsequent PR.
Test results
Snapshot time for 1 million keys:
The reads and writes mentioned above are during the snapshotting process on the actual store object.
Few improvements that can be done: