Feature flag for use of committing client in cometbft #9224
Assignees
Labels
Comments
|
It's been noted that having a new cosmos-sdk tag will change error stack traces. If a swingset call into Cosmos gets and error that returns a stack trace, and if that stack trace is eventually put into consensus state, then this becomes a potential consensus-breaking change. |
This was referenced Apr 11, 2024
|
Note that without further ceremony, the flag can be set by adding the following line to the top-level section in |
This was referenced Apr 18, 2024
mergify bot
added a commit
that referenced
this issue
Apr 18, 2024
refs: #9224 ## Description Upgrade our cosmos-sdk fork and cosmos/iavl to pick up bug fixes and features. ### Security Considerations See upgrade-15 notes. ### Scaling Considerations N/A ### Documentation Considerations N/A - internal changes ### Testing Considerations See upgrade-15 notes. ### Upgrade Considerations N/A
This was referenced Apr 19, 2024
mergify bot
added a commit
that referenced
this issue
Apr 19, 2024
closes: #9224 ## Description Pick up new version of cosmos-sdk that includes more tests and documentation for the abci client type setting. ### Security Considerations N/A ### Scaling Considerations If the setting is used to revert to the "local" client, query throughput drops dramatically. ### Documentation Considerations Docs added to TOML template, but this is an expert-level switch and no one should use it without a good understanding of the impact. ### Testing Considerations N/A ### Upgrade Considerations N/A
gibson042
pushed a commit
that referenced
this issue
Apr 19, 2024
refs: #9224 ## Description Upgrade our cosmos-sdk fork and cosmos/iavl to pick up bug fixes and features. ### Security Considerations See upgrade-15 notes. ### Scaling Considerations N/A ### Documentation Considerations N/A - internal changes ### Testing Considerations See upgrade-15 notes. ### Upgrade Considerations N/A
gibson042
pushed a commit
that referenced
this issue
Apr 19, 2024
closes: #9224 ## Description Pick up new version of cosmos-sdk that includes more tests and documentation for the abci client type setting. ### Security Considerations N/A ### Scaling Considerations If the setting is used to revert to the "local" client, query throughput drops dramatically. ### Documentation Considerations Docs added to TOML template, but this is an expert-level switch and no one should use it without a good understanding of the impact. ### Testing Considerations N/A ### Upgrade Considerations N/A
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What is the Problem Being Solved?
Our chain spends more time executing blocks than other chains do. We suffered from reduced RPC throughput. As a solution, we forked tendermint v0.34.12 and added a
CommittingClienttype of connection which used read-write locks for concurrency control instead of the pure mutex locks of theLocalClient. Commit.However, the committing client is a dangerous feature, and a suspect in an ongoing production incident. For this incident and future ones, we'd like an easy way to disable committing client. This is not a consensus-breaking change, so a feature flag seems like the right approach.
Description of the Design
In
cosmos-sdk/server/start.gowe have theagd serversubcommand and the call to construct the committing client. This is the nexus to add a flag and switch on it to perhaps use a local client instead. Using a boolean brings up arguments of which orientation to use ("--dont-not-disable-feature=false"), whereas--consensus-client-type={local,committing}(defaulting to committing) is more self-documenting and future-proof.As we're targeting a patch release (or just pointing validators to a commit hash), we'll land this change on the
Agoricbranch of agoric-labs/cosmos-sdk, cherry-pick it ontoAgoric-v0.46.16.2.x, tagv0.46.16-alpha.agoric.2.2, then make a version of agoric-sdk using that tag.Security Considerations
N/A
Scaling Considerations
Local client was originally much slower than committing client, but we have not run performance tests recently.
If local client is still notably slower, operators may need to adjust provisioning accordingly, or segregate certain kinds of traffic.
Test Plan
Unit tests bring it to parity with other tests. Manual testing confirmed.
Upgrade Considerations
N/A
The text was updated successfully, but these errors were encountered: