fix: prune cmd should disable async pruning #22656
Conversation
yihuang
requested review from
cool-develope,
kocubinski,
a team,
julienrbrt and
JulianToledano
as code owners
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces several updates across multiple modules, including new features such as nested message support, a Linux-only backend for the crypto/keyring module, and enhancements for importing hex keys. It also includes improvements like removing redundant contexts in tests and bug fixes related to simulation tests and CLI queries. Additionally, new methods for resource management have been added to the Store structs, enhancing error handling and cleanup processes. The changelog format has been refined to improve clarity and versioning. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Warning Rate limit exceeded@yihuang has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 21 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
store/rootmulti/store.go (1)
1173-1181: LGTM! Consider improving error handling.The Close method implementation is well-structured with proper resource cleanup. However, it might be beneficial to log errors when they occur, as some errors might be silently combined.
Consider adding error logging:
func (rs *Store) Close() error { errs := make([]error, 0, len(rs.stores)) for _, store := range rs.stores { if closer, ok := store.(io.Closer); ok { - errs = append(errs, closer.Close()) + if err := closer.Close(); err != nil { + rs.logger.Error("failed to close store", "err", err) + errs = append(errs, err) + } } } return errors.Join(errs...) }🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 1175-1179: Iteration over map
Iteration over map may be a possible source of non-determinismCHANGELOG.md (3)
Line range hint
1-1: Add title to changelog fileThe changelog file should start with a clear title like "# Changelog" to follow standard markdown documentation practices.
+ # Changelog
Line range hint
13-15: Improve version header formattingThe version headers should follow a consistent format with release date. Consider using H2 (##) headers for versions.
- ## [v0.47.16](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.47.16) - 2024-09-20 + ## v0.47.16 (2024-09-20) + + [Release Link](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.47.16)
Line range hint
2558-2558: Add footer sectionThe changelog should have a footer section with links to related resources and contribution guidelines.
+ ## Additional Resources + + - [Contributing Guidelines](CONTRIBUTING.md) + - [Release Process](RELEASE_PROCESS.md) + - [Security Policy](SECURITY.md)
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)client/pruning/main.go(1 hunks)store/iavl/store.go(1 hunks)store/rootmulti/store.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
CHANGELOG.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
client/pruning/main.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
store/iavl/store.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
store/rootmulti/store.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
🪛 GitHub Check: CodeQL
store/rootmulti/store.go
[warning] 1175-1179: Iteration over map
Iteration over map may be a possible source of non-determinism
🔇 Additional comments (3)
client/pruning/main.go (1)
92-94: LGTM! Improved error handling pattern
The simplified error handling follows Go's idiomatic patterns and improves code readability.
store/iavl/store.go (1)
407-412: LGTM! Clean implementation of resource cleanup.
The Close method follows Go's best practices for resource cleanup:
- Safely handles cases where the tree doesn't implement
io.Closer - Properly propagates errors from the underlying Close operation
Let's verify the usage of this Close method across the codebase:
✅ Verification successful
Let me analyze the usage of Close() method in the codebase to verify if there are any potential resource leaks.
Let me check if there are any other Store implementations that might require cleanup.
Based on the analysis of the codebase, I can now provide a final assessment of the Close() method implementation.
Implementation is correct and follows best practices
The Close() method implementation in store/iavl/store.go is properly designed:
- It correctly implements the
io.Closerinterface by checking if the underlying tree implementsio.Closer - The type assertion pattern
closer, ok := st.tree.(io.Closer)is the idiomatic way to handle optional interface implementations - The method properly propagates any errors from the underlying Close operation
- The implementation is consistent with other Store implementations in the codebase that require cleanup
The verification shows that:
- The Store struct is used in contexts where proper cleanup is needed (e.g., database connections)
- The Close method is called in appropriate places, particularly in cleanup paths
- The pattern of checking for io.Closer implementation is used consistently across the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for proper cleanup of Store instances
# Look for patterns where Store is used but Close is not called
# Search for Store instantiations
rg "NewStore|LoadStore|UnsafeNewStore" -A 5
# Search for defer Store.Close() patterns
rg "defer.*Close\(\)"
Length of output: 99577
Script:
#!/bin/bash
# Search for Store struct implementations that embed io.Closer
ast-grep --pattern 'type Store struct {
$$$
io.Closer
$$$
}'
# Search for Store struct implementations that have Close method
ast-grep --pattern 'func ($x *Store) Close() $_ {
$$$
}'
# Search for direct usage of Store.Close()
rg "store\.Close\(\)" -A 2
Length of output: 277
Script:
#!/bin/bash
# Search for any Store interface definitions that might require Close
ast-grep --pattern 'type $x interface {
$$$
Close() $_
$$$
}'
# Search for any Store implementations that might require Close
rg "type.*Store.*interface" -A 10
# Look for any Close method implementations in store package
rg "func.*Close" store/
Length of output: 51300
store/rootmulti/store.go (1)
1175-1179: Non-deterministic map iteration is acceptable here.
The static analysis warning about non-deterministic map iteration can be safely ignored in this context because:
- All stores need to be closed regardless of order
- The order of error collection doesn't affect the functionality
- If deterministic error reporting becomes necessary, the existing
keysFromStoreKeyMaphelper function can be utilized
🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 1175-1179: Iteration over map
Iteration over map may be a possible source of non-determinism
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (7)
client/pruning/main.go (1)
92-98: Consider using defer for cleanupTo ensure cleanup happens even if pruning fails, consider using defer:
+ defer func() { + if closeErr := rootMultiStore.Close(); closeErr != nil { + if err == nil { + err = closeErr + } else { + // Log the close error if we already have a pruning error + logger.Error("failed to close store", "error", closeErr) + } + } + }() if err := rootMultiStore.PruneStores(pruningHeight); err != nil { return err } - // close will wait for async pruning process to finish - if err := rootMultiStore.Close(); err != nil { - return err - }store/iavl/store.go (1)
407-412: LGTM! Clean implementation of io.CloserThe implementation correctly handles resource cleanup by delegating to the underlying tree's Close method when available.
Consider adding a doc comment explaining the purpose:
+// Close implements io.Closer interface. It closes the underlying tree if it implements io.Closer, +// otherwise returns nil. This ensures proper cleanup of resources, particularly after async operations. func (st *Store) Close() error {store/rootmulti/store.go (1)
1173-1181: LGTM! Consider adding error logging.The Close() implementation correctly handles resource cleanup by closing all stores that implement io.Closer. The use of errors.Join for combining multiple errors is a good practice.
Consider logging individual errors before joining them, to help with debugging:
func (rs *Store) Close() error { errs := make([]error, 0, len(rs.stores)) for _, store := range rs.stores { if closer, ok := store.(io.Closer); ok { - errs = append(errs, closer.Close()) + if err := closer.Close(); err != nil { + rs.logger.Error("failed to close store", "err", err) + errs = append(errs, err) + } } } return errors.Join(errs...) }🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 1175-1179: Iteration over map
Iteration over map may be a possible source of non-determinismCHANGELOG.md (4)
Line range hint
1-1: Add version table of contents for easier navigationConsider adding a table of contents at the start of the changelog listing all versions with links for easier navigation through this extensive document.
Line range hint
785-785: Improve formatting consistency in breaking changes sectionThe breaking changes section uses inconsistent bullet point styles (sometimes * and sometimes -). Standardize on a single bullet point style throughout the document for better readability.
Line range hint
1452-1452: Fix typo in changelog entry"Typographical" is misspelled as "Typograhical" in the list of review areas.
Line range hint
2345-2345: Add missing version release dateThe release date for v0.44.0 is listed as "2021-XX-XX". This should be updated with the actual release date.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)client/pruning/main.go(1 hunks)store/iavl/store.go(1 hunks)store/rootmulti/store.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
CHANGELOG.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
client/pruning/main.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
store/iavl/store.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
store/rootmulti/store.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
🪛 GitHub Check: CodeQL
store/rootmulti/store.go
[warning] 1175-1179: Iteration over map
Iteration over map may be a possible source of non-determinism
🔇 Additional comments (4)
client/pruning/main.go (2)
92-94: LGTM: Clean error handling pattern
The simplified error handling follows Go's idiomatic patterns and improves code readability.
96-98: LGTM: Proper cleanup with async handling
The addition of Close() ensures that async pruning operations complete before the command exits, addressing the core objective of this PR.
store/iavl/store.go (1)
407-412: Verify integration with pruning operations
Since this PR aims to fix pruning command waiting for async pruning, let's verify the integration.
✅ Verification successful
Let me analyze the pruning-related code to verify the integration with the Close method.
Let me check how the pruning manager interacts with the store.
Store Close implementation is correctly integrated with pruning operations
The Close method in store/iavl/store.go is properly integrated with pruning operations for the following reasons:
- The pruning manager operates independently of the store's Close method, using a separate pruner interface for both state commitment (SC) and state storage (SS).
- The Close method correctly handles cleanup by:
- Checking if the underlying tree implements io.Closer
- Properly propagating the Close call to the tree if it's closeable
- Safely handling non-closeable trees by returning nil
- The pruning operations (Prune, PausePruning, ResumePruning) do not maintain any persistent resources that would require explicit cleanup through Close.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check how Close is used in pruning-related code
# Look for pruning-related files that should use Close
echo "Searching for pruning-related files..."
rg -l "pruning" --type go
# Check for Close usage in pruning operations
echo "Checking Close usage in pruning context..."
rg -B 5 -A 5 "Close\(\)" --type go
Length of output: 102805
Script:
#!/bin/bash
# Check for pruning-related code that interacts with Close
echo "Checking pruning manager code..."
rg -B 5 -A 5 "Close\(\)" store/pruning/
rg -B 5 -A 5 "Close\(\)" store/v2/pruning/
# Check for pruning manager tests
echo "Checking pruning manager tests..."
rg -B 5 -A 5 "Close" store/pruning/manager_test.go
rg -B 5 -A 5 "Close" store/v2/pruning/manager_test.go
# Check for store Close implementation
echo "Checking store Close implementation..."
rg -B 5 -A 5 "Close\(\)" store/iavl/store.go
Length of output: 1399
Script:
#!/bin/bash
# Check how pruning manager uses store
echo "Checking pruning manager store usage..."
rg -B 10 -A 10 "type Manager struct" store/pruning/manager.go
rg -B 10 -A 10 "type Manager struct" store/v2/pruning/manager.go
# Check pruning manager initialization
echo "Checking pruning manager initialization..."
rg -B 5 -A 10 "NewManager" store/pruning/manager.go
rg -B 5 -A 10 "NewManager" store/v2/pruning/manager.go
# Check store usage in pruning
echo "Checking store usage in pruning..."
rg -B 5 -A 10 "Prune" store/pruning/manager.go
rg -B 5 -A 10 "Prune" store/v2/pruning/manager.go
Length of output: 5636
store/rootmulti/store.go (1)
1175-1179: Non-deterministic map iteration is acceptable for cleanup.
The non-deterministic map iteration in the Close method is acceptable because:
- Resource cleanup doesn't require deterministic ordering
- All stores will be processed regardless of iteration order
- All errors are collected and joined
✅ Verification successful
Let me analyze the search results to verify if non-deterministic map iteration is acceptable for cleanup in this context.
Non-deterministic map iteration is acceptable for cleanup operations
The map iteration in the Close() method is safe because:
- The method's sole purpose is resource cleanup
- All stores are processed regardless of iteration order
- All errors are collected into a slice and joined together
- The order of cleanup doesn't affect the correctness or final state
- The implementation uses errors.Join() which handles multiple errors appropriately
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify if deterministic ordering is required elsewhere in the codebase
# by checking for any comments or documentation mentioning order requirements.
rg -i "order|deterministic" --type go
Length of output: 122332
Script:
#!/bin/bash
# Check if the stores map is used in any other critical operations where order matters
ast-grep --pattern 'rs.stores'
Length of output: 30
Script:
#!/bin/bash
# Search for the usage of stores map in the file to understand the context
rg -A 5 -B 5 "stores\s*:=\s*make\(map\[string\]" store/rootmulti/store.go
Length of output: 75
Script:
#!/bin/bash
# Look for the Close method implementation and surrounding context
rg -A 10 -B 10 "func.*Close\(\)" store/rootmulti/store.go
Length of output: 524
🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 1175-1179: Iteration over map
Iteration over map may be a possible source of non-determinism
yihuang
changed the title
yihuang
requested review from
facundomedica,
aaronc and
testinginprod
as code owners
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
baseapp/options.go (1)
89-92: Enhance function documentationThe implementation looks good and follows the established patterns. However, the documentation could be more descriptive to explain the impact of sync vs async pruning.
Consider expanding the documentation:
-// SetIAVLSyncPruning set sync/async pruning in the IAVL store. +// SetIAVLSyncPruning configures whether IAVL store pruning should be synchronous or asynchronous. +// When syncPruning is true, pruning operations will complete before returning control, +// ensuring all pruned data is immediately removed. When false, pruning occurs +// asynchronously in the background.CHANGELOG.md (4)
Line range hint
1-1: Add title to changelog fileThe changelog file should start with a clear title like "# Changelog" to follow documentation best practices.
+ # Changelog
Line range hint
3-3: Standardize version date formatThe date format should be consistent across all versions. Currently some versions use YYYY-MM-DD while others use Month DD, YYYY.
Use ISO 8601 date format (YYYY-MM-DD) consistently for all version dates.
Line range hint
2-2: Add changelog guidelines sectionConsider adding a "Changelog Guidelines" section at the top to document the expected format and style for changelog entries.
Add guidelines for:
- Entry format and style
- Section organization
- Link formatting
- Version numbering
Line range hint
1-2000: Fix markdown formatting inconsistenciesThere are several formatting inconsistencies throughout the file:
- Inconsistent indentation in lists
- Mixed usage of * and - for bullet points
- Inconsistent spacing around headers
Standardize on:
- Using - for all bullet points
- Consistent indentation (2 spaces)
- One blank line before headers
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (8)
CHANGELOG.md(1 hunks)baseapp/options.go(1 hunks)client/pruning/main.go(1 hunks)server/start.go(1 hunks)server/util.go(1 hunks)store/iavl/store.go(1 hunks)store/rootmulti/store.go(3 hunks)store/types/store.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- client/pruning/main.go
- store/iavl/store.go
- store/rootmulti/store.go
🧰 Additional context used
📓 Path-based instructions (5)
CHANGELOG.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
baseapp/options.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
server/start.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
server/util.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
store/types/store.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
🔇 Additional comments (3)
store/types/store.go (1)
224-226: Improve method documentation and verify implementation
The method documentation should be more descriptive about the impact of the sync parameter. Consider adding details about the behavior differences between sync and async pruning modes.
Let's verify that all implementations of CommitMultiStore have implemented this new method:
✅ Verification successful
Let's search more specifically for CommitMultiStore implementations and their relationship with the new method.
Let's check the implementation details and usage context with one final verification.
Documentation improvement needed for SetIAVLSyncPruning
The method is properly implemented in the rootmulti Store and correctly integrated into the BaseApp options, but the documentation should be enhanced to:
- Explain that
sync=trueenables synchronous pruning whilesync=falseenables asynchronous pruning - Mention that this setting affects the IAVL store's pruning behavior
- Reference that this can be configured through BaseApp options
Example documentation:
// SetIAVLSyncPruning sets the pruning mode for IAVL stores.
// When sync is true, pruning is performed synchronously during commits.
// When sync is false, pruning is performed asynchronously in the background.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all implementations of CommitMultiStore interface and verify they implement SetIAVLSyncPruning
# Find all Go files that might implement CommitMultiStore
echo "Searching for potential CommitMultiStore implementations..."
rg -l "type.*Store.*struct" --type go
# Find the actual method implementations
echo -e "\nSearching for SetIAVLSyncPruning implementations..."
ast-grep --pattern 'func ($x *$t) SetIAVLSyncPruning(sync bool) { $$$ }'
Length of output: 2395
Script:
#!/bin/bash
# Find direct implementations of CommitMultiStore
echo "Searching for CommitMultiStore interface implementations..."
rg -B 2 -A 2 "type.*struct.*{.*}" --type go | grep -B 2 -A 2 "implements.*CommitMultiStore"
# Search for rootmulti store which is likely the main implementation
echo -e "\nChecking rootmulti store implementation..."
rg "SetIAVLSyncPruning" store/rootmulti/store.go
# Look for any SetIAVLSyncPruning references
echo -e "\nSearching for all SetIAVLSyncPruning references..."
rg "SetIAVLSyncPruning" --type go
Length of output: 1249
Script:
#!/bin/bash
# Check the Store implementation in rootmulti
echo "Checking Store implementation in rootmulti..."
rg -B 5 -A 5 "func \(rs \*Store\) SetIAVLSyncPruning" store/rootmulti/store.go
# Check IAVL store implementation
echo -e "\nChecking IAVL store implementation..."
rg -B 5 -A 5 "syncPruning" store/iavl/store.go
# Check BaseApp usage
echo -e "\nChecking BaseApp usage..."
rg -B 5 -A 5 "SetIAVLSyncPruning" baseapp/options.go
Length of output: 2364
server/util.go (1)
558-558: LGTM! Verify flag definition.
The addition of IAVL sync pruning option follows the established pattern of other baseapp options. The implementation is correct and aligns with the PR objective to control pruning synchronization.
Let's verify the flag constant is properly defined:
✅ Verification successful
Flag constant is properly defined
The FlagIAVLSyncPruning constant is correctly defined in server/start.go as "iavl-sync-pruning", which follows the naming convention for flag constants in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if FlagIAVLSyncPruning is properly defined
# Expected: Find the flag definition in server package
# Search for the flag definition
rg "FlagIAVLSyncPruning.*=.*" --type go
Length of output: 104
server/start.go (1)
82-82: LGTM! Flag constant follows established patterns.
The new FlagIAVLSyncPruning constant follows the existing naming conventions and is properly placed with other IAVL-related flags.
Let's verify the flag usage across the codebase:
✅ Verification successful
Flag is properly integrated and used in the codebase
The FlagIAVLSyncPruning constant is correctly integrated into the codebase:
- Used in
server/util.goto configure IAVL store pruning behavior viabaseapp.SetIAVLSyncPruning - Proper implementation exists in
store/types,store/rootmulti, andbaseapp/options.go - Used alongside other pruning-related flags in the pruning command context
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of the new IAVL sync pruning flag
# Test: Search for flag usage in code. Expect: Usage in pruning command and configuration.
rg "iavl-sync-pruning" --type go
Length of output: 95
Script:
#!/bin/bash
# Let's search for any references to the FlagIAVLSyncPruning constant
rg "FlagIAVLSyncPruning" --type go -A 3
Length of output: 716
Script:
#!/bin/bash
# Let's also check for any pruning-related command or configuration code
rg "func.*[Pp]rune|[Pp]runing" --type go -A 5
Length of output: 101956
Script:
#!/bin/bash
# Let's check for the actual usage of the flag in the codebase
rg "SetIAVLSyncPruning" --type go -A 3
Length of output: 1184
Script:
#!/bin/bash
# Let's also check for any pruning command implementation
rg "cmd.*[Pp]rune" --type go -A 5
Length of output: 1766
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
server/mock/store.go (1)
145-147: LGTM! Consider adding interface documentation.The implementation follows the established pattern for mock methods in this file. However, for better maintainability, consider adding a comment documenting the interface contract, especially since this is part of a larger pruning-related change.
Add documentation like this:
+// SetIAVLSyncPruning implements the MultiStore interface by providing a mock +// that panics when called. This setting controls whether IAVL pruning should +// be performed synchronously. func (ms multiStore) SetIAVLSyncPruning(syncPruning bool) { panic("not implemented") }
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (1)
server/mock/store.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
server/mock/store.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
| func (ms multiStore) SetIAVLSyncPruning(syncPruning bool) { | ||
| panic("not implemented") | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Missing implementation in CacheMultiStore
The SetIAVLSyncPruning method is defined in the MultiStore interface and needs to be implemented by all implementations. While rootmulti.Store has a proper implementation, cachemulti.Store is missing this method. The mock implementation in server/mock/store.go correctly panics for unimplemented functionality, but store/cachemulti/store.go needs to be updated to implement this interface method.
store/cachemulti/store.go: Add implementation ofSetIAVLSyncPruning(bool)method
🔗 Analysis chain
Verify interface implementation across the codebase
Since this adds a new method to the MultiStore interface implementation, we should verify that all other implementations are updated accordingly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for all MultiStore interface implementations
ast-grep --pattern 'type $_ struct {
$$$
}
var _ storetypes.MultiStore = $_'
Length of output: 81
Script:
#!/bin/bash
# Let's try a different approach to find MultiStore implementations
# First, let's find the interface definition
rg "type\s+MultiStore\s+interface" -A 10
# Then search for struct types that might implement it
rg "type.*Store.*struct"
# And also search for explicit interface checks
rg "var\s+_\s+.*MultiStore\s+=\s+"
Length of output: 7067
Script:
#!/bin/bash
# Let's check the implementation of the SetIAVLSyncPruning method in both implementations we found
# Check store/cachemulti/store.go
rg "SetIAVLSyncPruning" store/cachemulti/store.go -A 3
# Check store/rootmulti/store.go
rg "SetIAVLSyncPruning" store/rootmulti/store.go -A 3
# Let's also check the interface definition to see if this is a new method
rg "SetIAVLSyncPruning" store/types/store.go -A 3
Length of output: 445
Description
When async pruning enabled, prune cmd should wait for prune process to finish.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation