[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
190a462 to
5953601
Compare
08074ba to
4f1f09d
Compare
5953601 to
4a54b42
Compare
0955ccb to
8177757
Compare
1ea0be7 to
ea43efc
Compare
8177757 to
efd0192
Compare
ea43efc to
a835d1f
Compare
efd0192 to
fba2a36
Compare
d7bd6df to
96c606b
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") | ||
| } |
96c606b to
5a0946c
Compare
dd25d6c to
3e9115e
Compare
2 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.