Add support for incrementally persisting the active block in the peers bootstrapper as a snapshot #903
Conversation
| // SetIncremental sets whether this bootstrap should be an incremental | ||
| // that saves intermediate results to durable storage or not. | ||
| SetIncremental(value bool) RunOptions | ||
| // SetIncrementalConfig sets incremental configuration for this bootstrap. |
There was a problem hiding this comment.
hm maybe add a Validate() here too and ensure the file set types are only ones we support?
There was a problem hiding this comment.
also, maybe expose a function to check supported Incremental configurations from each bootstrapper and then have the processoptions validate the configuration + bootstrappers together
There was a problem hiding this comment.
I can add the validate from the first comment but I don't really understand the second comment.
There was a problem hiding this comment.
I wonder if instead of having a validate method we could just make it impossible to construct a PersistConfig incorrectly?
| // that saves intermediate results to durable storage or not. | ||
| Incremental() bool | ||
| // IncrementalConfig returns the incremental configuration for this bootstrap. | ||
| IncrementalConfig() IncrementalConfig |
There was a problem hiding this comment.
maybe time to rename this guy to PersistConfig too (talked to @robskillington, that's the best we could come up with)
…hing if node is Available or Leaving for that shard and add uninitialized source
richardartoul
force-pushed
the
ra/incremental-peers-snapshot
branch
from
ce64dff to
4579370
Compare
richardartoul
changed the title
Codecov Report
@@ Coverage Diff @@
## master #903 +/- ##
==========================================
+ Coverage 78.62% 78.64% +0.01%
==========================================
Files 404 404
Lines 34257 34286 +29
==========================================
+ Hits 26934 26963 +29
+ Misses 5478 5474 -4
- Partials 1845 1849 +4
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #903 +/- ##
==========================================
+ Coverage 78.62% 78.63% +<.01%
==========================================
Files 404 404
Lines 34257 34286 +29
==========================================
+ Hits 26934 26960 +26
+ Misses 5478 5475 -3
- Partials 1845 1851 +6
Continue to review full report at Codecov.
|
| defaultIncremental = false | ||
| var ( | ||
| // defaultPersistConfig declares the intent to by default to perform | ||
| // a bootstrap without persistence enabled. |
There was a problem hiding this comment.
hm you say without here but have Enabled: true below?
| // the data it streamed from its peers from the commit log as soon as | ||
| // the bootstrapping process completes. | ||
| require.NoError(t, setups[1].stopServer()) | ||
| startServerWithNewInspection(t, opts, setups[1]) |
There was a problem hiding this comment.
do you need to reset the bootstrappers here - i.e. ensure it doesn't use peers again?
There was a problem hiding this comment.
nah startServerWithNewInspection completely reconstructs the bootstrappers
| @@ -1155,6 +1157,11 @@ func (s *fileSystemSource) bootstrapFromIndexPersistedBlocks( | |||
| return res, nil | |||
| } | |||
|
|
|||
| func (s *fileSystemSource) shouldPersist(runOpts bootstrap.RunOptions) bool { | |||
| persistConfig := runOpts.PersistConfig() | |||
| return persistConfig.Enabled && persistConfig.FileSetType == persist.FileSetFlushType | |||
Addresses #900 and depends on #894