[Storehouse] 009 Payloadless factory and localnet - #8581
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
zhangchiqing
changed the title
zhangchiqing
changed the title
zhangchiqing
changed the title
| // | ||
| // triggerCheckpoint is a runtime control signal to trigger checkpoint on | ||
| // next segment finish (ignored by the remote client; can be nil). | ||
| func NewPayloadlessLedger(config Config, triggerCheckpoint *atomic.Bool) (ledger.PayloadlessLedger, error) { |
There was a problem hiding this comment.
mirrored from NewLedger
| } | ||
|
|
||
| // Use factory to create ledger with internal compactor | ||
| factory := complete.NewLocalLedgerFactory( |
There was a problem hiding this comment.
Previously this went through a LocalLedgerFactory indirection. That factory's only job was to construct the ledger storage, and nothing else referenced it, so it added no value. We've inlined it here and return the ledger storage directly. Therefore, the ledger/complete/factory.go file was removed in this PR. The payloadless path (newLocalPayloadlessLedger) follows the same pattern.
| Str("ledger_service_addr", config.LedgerServiceAddr). | ||
| Msg("using remote ledger service") | ||
|
|
||
| factory := remote.NewRemoteLedgerFactory( |
There was a problem hiding this comment.
Previously this went through a RemoteLedgerFactory that only constructed the remote client. The factory abstraction added no value, so we removed it and create the remote ledger client directly here. And the same pattern applied to newRemotePayloadlessLedger.
059892d to
08074ba
Compare
08074ba to
4f1f09d
Compare
0955ccb to
8177757
Compare
8177757 to
efd0192
Compare
efd0192 to
fba2a36
Compare
fba2a36 to
dd25d6c
Compare
|
|
||
| return dockerServices | ||
| } | ||
|
|
There was a problem hiding this comment.
This trailing blank line at EOF fails gofmt. Because the file is in the integration/ module, the main-module make lint passes and won't catch it locally, but integration CI lint will fail.
| // or a newer numbered one written by the compactor), so no conversion is | ||
| // needed at runtime. The os.Stat guard makes a re-run of `make bootstrap` | ||
| // idempotent and avoids ConvertCheckpointV6ToV7's "output exists" rejection. | ||
| if payloadless { |
There was a problem hiding this comment.
nit: this conversion isn't guarded on the V6 source existing. If neither a V6 nor a V7 root checkpoint is present, ConvertCheckpointV6ToV7 is still called and panics on the missing source.
| if payloadless { | ||
| service.Command = append(service.Command, "--payloadless") | ||
| } | ||
|
|
||
| // Payloadless mode requires storehouse to store the actual payloads | ||
| // (the trie only stores payload hashes) | ||
| if payloadless { | ||
| service.Command = append(service.Command, "--enable-storehouse") | ||
| } |
dd25d6c to
3e9115e
Compare
5c5ad9b to
a46a44d
Compare
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
a46a44d to
a7a441c
Compare
a7a441c to
11010fb
Compare
11010fb to
07b2356
Compare
07b2356 to
2b01b60
Compare
2b01b60 to
3df3fe1
Compare
3df3fe1 to
409633a
Compare
This comment has been minimized.
This comment has been minimized.
409633a to
5a1e23d
Compare
3 participants
The previous PR #8598 can startup a payloadless mode EN with a payloadless checkpoint file, and replay wal files to resume execution. But the localnet doesn't work, because localnet is bootstrapping with a v6 checkpoint, no v7 checkpoint. So a automatic conversion from v6 to v7 checkpoint is required during bootstrapping. And after this PR, we can startup a payloadless mode EN in localnet.