chore: backport the pruning fix PR to v1.1.x#910
Conversation
czarcas7ic
left a comment
There was a problem hiding this comment.
Aside from the test failure in CI, I have tested this on Osmosis mainnet and have had positive results.
| return err | ||
| } | ||
| if err := b.batch.Close(); err != nil { | ||
| b.mtx.Unlock() |
There was a problem hiding this comment.
how come there is a need for the extra mutexs?
There was a problem hiding this comment.
it is related to the legacy node pruning, it runs in background
|
can you add a changelog since its changing things |
| leftNode *Node | ||
| rightNode *Node | ||
| subtreeHeight int8 | ||
| isLegacy bool |
There was a problem hiding this comment.
| isLegacy bool | |
| isV0Node bool |
or similar. legacy is ambiguous?
There was a problem hiding this comment.
we are using the legacy term from design stage, just worried if there are many things have to change
| // 1.1.0-<version of the current live state>. Returns error if storage version is incorrect or on | ||
| // db error, nil otherwise. Requires changes to be committed after to be persisted. | ||
| func (ndb *nodeDB) setFastStorageVersionToBatch(latestVersion int64) error { | ||
| func (ndb *nodeDB) SetFastStorageVersionToBatch(latestVersion int64) error { |
There was a problem hiding this comment.
why is this now exported?
There was a problem hiding this comment.
all APIs which are used outside of nodeDB is exported, the function export rules are messy in the current imp, but just following the current imp
|
I wanted to point out that when I implemented this IAVL branch on cosmos sdk, some tests started failing (one test that I checked is exactly the same as the test in v0.50, so it seems to not be due to me being on v0.47). These tests are: They seem to be non deterministic as well, so if I rerun there are likely more than just these. |
we will look into this prior to tag, thanks for the heads up |
@czarcas7ic please check the new commit |
|
Tests now pass @tac0turtle @cool-develope |
ref: #909