Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: nested iterator on cache store #14798

Merged
merged 3 commits into from
Jan 27, 2023
Merged

fix: nested iterator on cache store #14798

merged 3 commits into from
Jan 27, 2023

Conversation

yihuang
Copy link
Collaborator

@yihuang yihuang commented Jan 27, 2023 •
edited
Loading

Description

Closes: #14786
Solution:

  • do a copy on btree before iteration

minimal change to fix the iteration issue compared with #14378, I guess it's not consensus breaking because it fix a bug which is probably not triggered on production yet?

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...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • followed the guidelines for building modules
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers 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.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

@yihuang
fix: nested iterator on cache store
cfe543a 
Closes: cosmos#14786
Solution:
- do a copy on btree before iteration
@yihuang yihuang requested a review from a team as a code owner January 27, 2023 03:56
yihuang
yihuang commented Jan 27, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@julienrbrt julienrbrt added backport/0.45.x backport/0.46.x PR scheduled for inclusion in the v0.46's next stable release backport/v0.47.x PR scheduled for inclusion in the v0.47's next stable release labels Jan 27, 2023
tac0turtle
tac0turtle approved these changes Jan 27, 2023
View reviewed changes
Copy link
Member

@tac0turtle tac0turtle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UtACK. Before final release, we will test on a few mainnets

@julienrbrt
Copy link
Member

julienrbrt commented Jan 27, 2023 •
edited
Loading

UtACK. Before final release, we will test on a few mainnets

Are we aware of multiple mainnet needing this behavior (except Provenance) ?

@julienrbrt
Copy link
Member

julienrbrt commented Jan 27, 2023 •
edited
Loading

I think the PR was included in all versions. Shouldn't this target main first? Oh it's already on main.

@yihuang
Copy link
Collaborator Author

yihuang commented Jan 27, 2023

I think the PR was included in all versions. Shouldn't this target main first?

no, main and 0.47 has #14378, it's not backport-ed because of consensus breaking.

@julienrbrt
Copy link
Member

I don't think v0.47 has #14378 🤔

@julienrbrt
Copy link
Member

Confirming that v0.47 does not have #14378. Should we backport this or #14378 to v0.47 then?

@julienrbrt julienrbrt removed the backport/0.46.x PR scheduled for inclusion in the v0.46's next stable release label Jan 27, 2023
@yihuang
Copy link
Collaborator Author

yihuang commented Jan 27, 2023

Confirming that v0.47 does not have #14378. Should we backport this or #14378 to v0.47 then?

Yeah, I think we should

@julienrbrt julienrbrt removed the backport/v0.47.x PR scheduled for inclusion in the v0.47's next stable release label Jan 27, 2023
@julienrbrt julienrbrt mentioned this pull request Jan 27, 2023
Merged
19 tasks
@julienrbrt julienrbrt enabled auto-merge (squash) January 27, 2023 17:09
julienrbrt
julienrbrt approved these changes Jan 27, 2023
View reviewed changes
Copy link
Member

@julienrbrt julienrbrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@julienrbrt julienrbrt merged commit 76d47b4 into cosmos:release/v0.46.x Jan 27, 2023
@julienrbrt
Copy link
Member

@Mergifyio backport release/v0.45.x

@mergify mergify bot mentioned this pull request Jan 27, 2023
Merged
mergify bot pushed a commit that referenced this pull request Jan 27, 2023
@yihuang @mergify
fix: nested iterator on cache store (#14798)
e7f3665 
(cherry picked from commit 76d47b4)

# Conflicts:
#	CHANGELOG.md
@mergify
Copy link
Contributor

mergify bot commented Jan 27, 2023

backport release/v0.45.x

✅ Backports have been created

tac0turtle pushed a commit that referenced this pull request Jan 27, 2023
@mergify
fix: nested iterator on cache store (backport #14798) (#14823)
f96336f
@alexanderbez alexanderbez mentioned this pull request Jan 28, 2023
Closed
@tac0turtle
Copy link
Member

How come this isn't backport to 0.46? that is the release used by province

@julienrbrt
Copy link
Member

How come this isn't backport to 0.46? that is the release used by province

The PR targets 0.46 :D

@yihuang yihuang deleted the fix-cache-store branch January 29, 2023 01:31
@tac0turtle
Copy link
Member

Oh missed that lol thanks

@fedekunze fedekunze mentioned this pull request Feb 9, 2023
Merged
19 tasks
SpicyLemon added a commit to provenance-io/cosmos-sdk that referenced this pull request Feb 16, 2023
@SpicyLemon @mergify
@likhita-809 @yihuang @julienrbrt @tac0turtle @mmsqe @robert-zaremba @elias-orijtech @facundomedica
Backport: #505 (v0.46.9) #517 (v0.46.10) and #510 (sanction Tx CLI) (#…
cf6f99f 
…518)

* fix: Fix Evidence CLI query flag parsing (backport cosmos#13458) (cosmos#14799)

Co-authored-by: Likhita Polavarapu <[email protected]>
Co-authored-by: HuangYi <[email protected]>
Co-authored-by: Julien Robert <[email protected]>

* fix: nested iterator on cache store (cosmos#14798)

* chore: update IAVL  (backport cosmos#14830) (cosmos#14835)

Co-authored-by: Marko <[email protected]>

* Mark v0.46.8-pio-2 in the changelog and release notes.

* feat: add config `iavl-lazy-loading` to enable lazy loading of iavl store (backport cosmos#14189) (cosmos#14862)

Co-authored-by: yihuang <[email protected]>
Co-authored-by: marbar3778 <[email protected]>

* chore: bump IAVL to v0.19.5 (backport cosmos#14889) (cosmos#14892)

Co-authored-by: Julien Robert <[email protected]>

* build(deps): bump tendermint to v0.34.25 (cosmos#14903)

* build(deps): bump tendermint to v0.34.26 (cosmos#14911)

* fix: return named error for write validators (backport cosmos#14919) (cosmos#14921)

Co-authored-by: mmsqe <[email protected]>
Co-authored-by: Julien Robert <[email protected]>

* Bump Tendermint to v0.34.25 (from v0.34.24). (#498)

* Fix a few listener proto comments. (#499)

* build(deps): bump btcd in v0.46 (cosmos#14846)

Co-authored-by: yihuang <[email protected]>
Co-authored-by: Julien Robert <[email protected]>
Co-authored-by: Elias Naur <[email protected]>
Co-authored-by: Marko <[email protected]>

* ci: update release slack channel and algolia api key (backport cosmos#14942) (cosmos#14945)

Co-authored-by: Julien Robert <[email protected]>

* feat: support profiling block replay during abci handshake (backport cosmos#14953) (cosmos#14964)

Co-authored-by: yihuang <[email protected]>
Co-authored-by: Julien Robert <[email protected]>

* Revert "Revert cosmos#14349 - cache iterator optimizations (#444)"

This reverts commit cd9d192.

* Mark v0.46.8-pio-3 in the changelog and release notes.

* fix: avoid broadcasting tx through query (backport cosmos#15044) (cosmos#15047)

Co-authored-by: Facundo Medica <[email protected]>
Co-authored-by: Julien Robert <[email protected]>

* Add CLI commands for sanction Tx gov props (#510)

* Fix the changelog.

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Likhita Polavarapu <[email protected]>
Co-authored-by: HuangYi <[email protected]>
Co-authored-by: Julien Robert <[email protected]>
Co-authored-by: Marko <[email protected]>
Co-authored-by: mmsqe <[email protected]>
Co-authored-by: Robert Zaremba <[email protected]>
Co-authored-by: Elias Naur <[email protected]>
Co-authored-by: Facundo Medica <[email protected]>
ankurdotb pushed a commit to cheqd/cosmos-sdk that referenced this pull request Feb 28, 2023
@yihuang @ankurdotb
fix: nested iterator on cache store (cosmos#14798)
8af020d
@faddat faddat mentioned this pull request Mar 23, 2023
Closed
19 tasks
@LampardNguyen234 LampardNguyen234 mentioned this pull request Aug 15, 2023
Merged
@faddat faddat mentioned this pull request Dec 11, 2023
Draft
19 tasks
JeancarloBarrios pushed a commit to agoric-labs/cosmos-sdk that referenced this pull request Sep 28, 2024
@mergify @JeancarloBarrios
fix: nested iterator on cache store (backport cosmos#14798) (cosmos#1…
da06670 
…4823)
JeancarloBarrios pushed a commit to agoric-labs/cosmos-sdk that referenced this pull request Sep 28, 2024
@yihuang @JeancarloBarrios
fix: nested iterator on cache store (cosmos#14798)
3a3aa8f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julienrbrt julienrbrt julienrbrt approved these changes

@tac0turtle tac0turtle tac0turtle approved these changes

Assignees
No one assigned
Labels
C:Store
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@yihuang @julienrbrt @tac0turtle