ADR-038 Part 1: WriteListener, listen.KVStore, MultiStore and KVStore updates #8551

i-norden · 2021-02-09T21:19:54Z

Description

Hi all 👋 this PR introduces the first stage of changes to support ADR-038 state listening. I've gone ahead and split this ADR implementation up into three segments to hopefully improve the review process. This first segment introduces the low level changes to the MultiStore and KVStore interfaces and implementations, the new WriteListener types, and the new listen.KVStore type.

In the next segment I'll introduce the StreamingService interface and an implementation that writes out to files, it's integration and configuration at the BaseApp level, as well as example integration into SimApp.

In the third segment I'll introduce an auxiliary service for consuming those files and serving the data remotely with filtering/search capabilities.

This work is towards satisfying ADR-038

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Code follows the module structure standards.
Wrote unit and integration tests
Updated relevant documentation (docs/) or specification (x/<module>/spec/)
Added relevant godoc comments.
Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer
Review Codecov Report in the comment section below once CI passes

codecov · 2021-02-09T21:31:39Z

Codecov Report

Merging #8551 (0e5614d) into master (f8f52c6) will increase coverage by 0.09%.
The diff coverage is 84.72%.

@@            Coverage Diff             @@
##           master    #8551      +/-   ##
==========================================
+ Coverage   58.87%   58.96%   +0.09%     
==========================================
  Files         573      575       +2     
  Lines       32091    32159      +68     
==========================================
+ Hits        18893    18963      +70     
+ Misses      10987    10982       -5     
- Partials     2211     2214       +3

Impacted Files	Coverage Δ
server/mock/store.go	`22.44% <0.00%> (-2.00%)`	⬇️
store/cachekv/store.go	`87.23% <0.00%> (-0.94%)`	⬇️
store/prefix/store.go	`86.36% <0.00%> (-1.33%)`	⬇️
store/types/store.go	`60.00% <ø> (ø)`
store/types/listening.go	`71.42% <71.42%> (ø)`
store/listenkv/store.go	`96.87% <96.87%> (ø)`
store/dbadapter/store.go	`100.00% <100.00%> (+8.33%)`	⬆️
store/gaskv/store.go	`100.00% <100.00%> (ø)`
store/iavl/store.go	`72.08% <100.00%> (+1.16%)`	⬆️
store/mem/store.go	`81.81% <100.00%> (+21.81%)`	⬆️
... and 8 more

i-norden · 2021-02-10T17:17:00Z

Fixing codecov today

amaury1093

overall lgtm, nice job! Put a couple of questions in the comments

store/cachekv/store.go

store/rootmulti/store.go

amaury1093 · 2021-02-24T12:25:27Z

store/types/listening.go

+}
+
+// OnWrite satisfies the WriteListener interface by writing length-prefixed protobuf encoded StoreKVPairs
+func (wl *StoreKVPairWriteListener) OnWrite(storeKey StoreKey, set bool, key []byte, value []byte) {


The error is silently ignored here. Would it make sense to update the OnWrite interface to add an error? The listener then decides what to do on error.

Yeah I think that makes sense! OnWrite is only called from within the listen.KvStore, we can return it to be handled there but the methods it is called within (Set and Delete) don't return an error themselves so it can't bubble up any further than that (without changing the existing KVStore interface methods).

My only hesitation is that there doesn't seem to be much precedent for initializing a KVStore with a logger to log from within that level.

Ah, I see, so Set and Delete will anyways silently ignore those errors.

Yeah introducing a logger there is a bigger refactor. I still think adding error on OnWrite is useful, but if you think it's superfluous we can remove it too.

I went ahead and added it to OnWrite! That way we don't have to break the interface if we want to log down the road.

store/types/listening.go

store/rootmulti/store_test.go

amaury1093

ACK, thanks!

i-norden · 2021-02-25T22:52:09Z

Thank you for the review @AmauryM!

… from running 'make proto-gen'

aaronc · 2021-03-30T19:47:16Z

Is this ready to be merged @i-norden ? Btw no need to force push, you can just merge.

i-norden · 2021-03-30T20:13:05Z

Hi @aaronc, yeah this is ready to merge! Aye, I've gotten in the (bad?) habit of rebasing and force pushing to align with the base branch but I will start merging instead.

…entation, and configuration (#8664)  ## Description  Hello 👋 this PR introduces the second stage of changes to support [ADR-038](#8012) state listening. This is rebased on top of the [first segment](#8551), which introduces the low level changes to the MultiStore and KVStore interfaces and implementations, the new WriteListener types, and the new listen.KVStore type. In this segment we introduce the StreamingService interface, an implementation that writes out to files, and it's integration and configuration at the BaseApp level. The idea was to have the first segment reviewed independently first but if people think it is easier/more time efficient to review both at the same time then we could start here. Thanks! This work is towards satisfying [ADR-038](https://github.com/cosmos/cosmos-sdk/blob/master/docs/architecture/adr-038-state-listening.md) --- Before we can merge this PR, please make sure that all the following items have been checked off. If any of the checklist items are not applicable, please leave them but write a little note why. - [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [x] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. - [x] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md). - [x] Wrote unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [x] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`) - [x] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code). - [x] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` - [x] Re-reviewed `Files changed` in the Github PR explorer - [x] Review `Codecov Report` in the comment section below once CI passes

…entation, and configuration (cosmos#8664)   Hello 👋 this PR introduces the second stage of changes to support [ADR-038](cosmos#8012) state listening. This is rebased on top of the [first segment](cosmos#8551), which introduces the low level changes to the MultiStore and KVStore interfaces and implementations, the new WriteListener types, and the new listen.KVStore type. In this segment we introduce the StreamingService interface, an implementation that writes out to files, and it's integration and configuration at the BaseApp level. The idea was to have the first segment reviewed independently first but if people think it is easier/more time efficient to review both at the same time then we could start here. Thanks! This work is towards satisfying [ADR-038](https://github.com/cosmos/cosmos-sdk/blob/master/docs/architecture/adr-038-state-listening.md) --- Before we can merge this PR, please make sure that all the following items have been checked off. If any of the checklist items are not applicable, please leave them but write a little note why. - [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [x] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. - [x] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md). - [x] Wrote unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [x] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`) - [x] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code). - [x] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` - [x] Re-reviewed `Files changed` in the Github PR explorer - [x] Review `Codecov Report` in the comment section below once CI passes

…entation, and configuration (backport cosmos#8664)   Hello 👋 this PR introduces the second stage of changes to support [ADR-038](cosmos#8012) state listening. This is rebased on top of the [first segment](cosmos#8551), which introduces the low level changes to the MultiStore and KVStore interfaces and implementations, the new WriteListener types, and the new listen.KVStore type. In this segment we introduce the StreamingService interface, an implementation that writes out to files, and it's integration and configuration at the BaseApp level. The idea was to have the first segment reviewed independently first but if people think it is easier/more time efficient to review both at the same time then we could start here. Thanks! This work is towards satisfying [ADR-038](https://github.com/cosmos/cosmos-sdk/blob/master/docs/architecture/adr-038-state-listening.md) --- Before we can merge this PR, please make sure that all the following items have been checked off. If any of the checklist items are not applicable, please leave them but write a little note why. - [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [x] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. - [x] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md). - [x] Wrote unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [x] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`) - [x] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code). - [x] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` - [x] Re-reviewed `Files changed` in the Github PR explorer - [x] Review `Codecov Report` in the comment section below once CI passes

… updates (cosmos#8551) * StoreKVPair protobuf message definition and generated go types * store WriteListener * update MultiStore, CacheWrap, CacheWrapper interfaces * adjust KVStores to fit new CacheWrapper interface * new ListenKVStore * adjust multistores to fit new MultiStore interface and enable wrapping returned KVStores with the new ListenKVStore * typo fixes in adr * ListenKV Store test * update server mock KVStore and MultiStore * multistore unit test; fix multistore constructor * update changelog * fix bug identified in CI * improve codecov, minor fixes/adjustments * review fixes * review updates; flip set to delete in KVStorePair, updated proto-docs from running 'make proto-gen'

…entation, and configuration (cosmos#8664)   Hello 👋 this PR introduces the second stage of changes to support [ADR-038](cosmos#8012) state listening. This is rebased on top of the [first segment](cosmos#8551), which introduces the low level changes to the MultiStore and KVStore interfaces and implementations, the new WriteListener types, and the new listen.KVStore type. In this segment we introduce the StreamingService interface, an implementation that writes out to files, and it's integration and configuration at the BaseApp level. The idea was to have the first segment reviewed independently first but if people think it is easier/more time efficient to review both at the same time then we could start here. Thanks! This work is towards satisfying [ADR-038](https://github.com/cosmos/cosmos-sdk/blob/master/docs/architecture/adr-038-state-listening.md) --- Before we can merge this PR, please make sure that all the following items have been checked off. If any of the checklist items are not applicable, please leave them but write a little note why. - [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [x] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. - [x] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md). - [x] Wrote unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [x] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`) - [x] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code). - [x] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` - [x] Re-reviewed `Files changed` in the Github PR explorer - [x] Review `Codecov Report` in the comment section below once CI passes

…entation, and configuration (backport #8664) (#13325) * feat: ADR-038 Part 2: StreamingService interface, file writing implementation, and configuration (backport #8664)   Hello 👋 this PR introduces the second stage of changes to support [ADR-038](#8012) state listening. This is rebased on top of the [first segment](#8551), which introduces the low level changes to the MultiStore and KVStore interfaces and implementations, the new WriteListener types, and the new listen.KVStore type. In this segment we introduce the StreamingService interface, an implementation that writes out to files, and it's integration and configuration at the BaseApp level. The idea was to have the first segment reviewed independently first but if people think it is easier/more time efficient to review both at the same time then we could start here. Thanks! This work is towards satisfying [ADR-038](https://github.com/cosmos/cosmos-sdk/blob/master/docs/architecture/adr-038-state-listening.md) --- Before we can merge this PR, please make sure that all the following items have been checked off. If any of the checklist items are not applicable, please leave them but write a little note why. - [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [x] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. - [x] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md). - [x] Wrote unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [x] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`) - [x] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code). - [x] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` - [x] Re-reviewed `Files changed` in the Github PR explorer - [x] Review `Codecov Report` in the comment section below once CI passes * Update CHANGELOG.md Co-authored-by: Ian Norden <iansnorden@gmail.com>

…entation, and configuration (#8664) (#13326) * feat: ADR-038 Part 2: StreamingService interface, file writing implementation, and configuration (#8664)   Hello 👋 this PR introduces the second stage of changes to support [ADR-038](#8012) state listening. This is rebased on top of the [first segment](#8551), which introduces the low level changes to the MultiStore and KVStore interfaces and implementations, the new WriteListener types, and the new listen.KVStore type. In this segment we introduce the StreamingService interface, an implementation that writes out to files, and it's integration and configuration at the BaseApp level. The idea was to have the first segment reviewed independently first but if people think it is easier/more time efficient to review both at the same time then we could start here. Thanks! This work is towards satisfying [ADR-038](https://github.com/cosmos/cosmos-sdk/blob/master/docs/architecture/adr-038-state-listening.md) --- Before we can merge this PR, please make sure that all the following items have been checked off. If any of the checklist items are not applicable, please leave them but write a little note why. - [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [x] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. - [x] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md). - [x] Wrote unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [x] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`) - [x] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code). - [x] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` - [x] Re-reviewed `Files changed` in the Github PR explorer - [x] Review `Codecov Report` in the comment section below once CI passes * fix lint * Update CHANGELOG.md Co-authored-by: Ian Norden <iansnorden@gmail.com>

…entation, and configuration (#8664) (#13326) * feat: ADR-038 Part 2: StreamingService interface, file writing implementation, and configuration (#8664)   Hello 👋 this PR introduces the second stage of changes to support [ADR-038](cosmos/cosmos-sdk#8012) state listening. This is rebased on top of the [first segment](cosmos/cosmos-sdk#8551), which introduces the low level changes to the MultiStore and KVStore interfaces and implementations, the new WriteListener types, and the new listen.KVStore type. In this segment we introduce the StreamingService interface, an implementation that writes out to files, and it's integration and configuration at the BaseApp level. The idea was to have the first segment reviewed independently first but if people think it is easier/more time efficient to review both at the same time then we could start here. Thanks! This work is towards satisfying [ADR-038](https://github.com/cosmos/cosmos-sdk/blob/master/docs/architecture/adr-038-state-listening.md)

…entation, and configuration (#8664) (#13326) * feat: ADR-038 Part 2: StreamingService interface, file writing implementation, and configuration (#8664)   Hello 👋 this PR introduces the second stage of changes to support [ADR-038](cosmos/cosmos-sdk#8012) state listening. This is rebased on top of the [first segment](cosmos/cosmos-sdk#8551), which introduces the low level changes to the MultiStore and KVStore interfaces and implementations, the new WriteListener types, and the new listen.KVStore type. In this segment we introduce the StreamingService interface, an implementation that writes out to files, and it's integration and configuration at the BaseApp level. The idea was to have the first segment reviewed independently first but if people think it is easier/more time efficient to review both at the same time then we could start here. Thanks! This work is towards satisfying [ADR-038](https://github.com/cosmos/cosmos-sdk/blob/master/docs/architecture/adr-038-state-listening.md) --- Before we can merge this PR, please make sure that all the following items have been checked off. If any of the checklist items are not applicable, please leave them but write a little note why. - [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [x] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. - [x] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md). - [x] Wrote unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [x] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`) - [x] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code). - [x] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` - [x] Re-reviewed `Files changed` in the Github PR explorer - [x] Review `Codecov Report` in the comment section below once CI passes * fix lint * Update CHANGELOG.md Co-authored-by: Ian Norden <iansnorden@gmail.com>

i-norden requested review from aaronc, alessio and alexanderbez as code owners February 9, 2021 21:19

i-norden force-pushed the adr038_writelistener branch 3 times, most recently from 0fa9a3d to b4889a3 Compare February 9, 2021 21:48

i-norden force-pushed the adr038_writelistener branch 3 times, most recently from 5b2730a to 2dbb9ec Compare February 17, 2021 17:29

i-norden force-pushed the adr038_writelistener branch 6 times, most recently from b576743 to 980d00f Compare February 22, 2021 18:17

i-norden mentioned this pull request Feb 22, 2021

feat: ADR-038 Part 2: StreamingService interface, file writing implementation, and configuration #8664

Merged

9 tasks

amaury1093 reviewed Feb 24, 2021

View reviewed changes

amaury1093 added the C:Store label Feb 24, 2021

i-norden force-pushed the adr038_writelistener branch from 980d00f to 75fb929 Compare February 24, 2021 21:28

amaury1093 approved these changes Feb 25, 2021

View reviewed changes

i-norden force-pushed the adr038_writelistener branch 2 times, most recently from 9972464 to b320f44 Compare February 25, 2021 21:13

i-norden force-pushed the adr038_writelistener branch 5 times, most recently from d1ab6a6 to eb068b1 Compare March 2, 2021 15:01

i-norden added 6 commits March 30, 2021 13:13

update server mock KVStore and MultiStore

1e80402

multistore unit test; fix multistore constructor

5211b28

update changelog

56a644e

fix bug identified in CI

bfb9f1e

improve codecov, minor fixes/adjustments

4283a9f

review fixes

8530013

i-norden force-pushed the adr038_writelistener branch from 26cc072 to 1928a2c Compare March 30, 2021 18:15

review updates; flip set to delete in KVStorePair, updated proto-docs…

0e5614d

… from running 'make proto-gen'

i-norden force-pushed the adr038_writelistener branch from 1928a2c to 0e5614d Compare March 30, 2021 18:17

aaronc merged commit feed37d into cosmos:master Mar 30, 2021

clevinson mentioned this pull request Aug 5, 2021

Postgres Indexing (ADR038) Implementation #9817

Closed

4 tasks

i-norden mentioned this pull request Sep 7, 2021

ADR-038 Implementation #10096

Closed

11 tasks

yihuang mentioned this pull request Sep 19, 2022

backport adr-038 streaming service to older versions #13324

Closed

yihuang mentioned this pull request Sep 19, 2022

feat: ADR-038 Part 2: StreamingService interface, file writing implementation, and configuration (backport #8664) #13325

Merged

28 tasks

yihuang mentioned this pull request Sep 19, 2022

feat: ADR-038 Part 2: StreamingService interface, file writing implementation, and configuration (#8664) #13326

Merged

28 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ADR-038 Part 1: WriteListener, listen.KVStore, MultiStore and KVStore updates #8551

ADR-038 Part 1: WriteListener, listen.KVStore, MultiStore and KVStore updates #8551

i-norden commented Feb 9, 2021 •

edited

Loading

codecov bot commented Feb 9, 2021 •

edited

Loading

i-norden commented Feb 10, 2021

amaury1093 left a comment

amaury1093 Feb 24, 2021

i-norden Feb 24, 2021 •

edited

Loading

i-norden Feb 24, 2021

amaury1093 Feb 25, 2021

i-norden Feb 25, 2021

amaury1093 left a comment

i-norden commented Feb 25, 2021

aaronc commented Mar 30, 2021

i-norden commented Mar 30, 2021

ADR-038 Part 1: WriteListener, listen.KVStore, MultiStore and KVStore updates #8551

ADR-038 Part 1: WriteListener, listen.KVStore, MultiStore and KVStore updates #8551

Conversation

i-norden commented Feb 9, 2021 • edited Loading

Description

codecov bot commented Feb 9, 2021 • edited Loading

Codecov Report

i-norden commented Feb 10, 2021

amaury1093 left a comment

Choose a reason for hiding this comment

amaury1093 Feb 24, 2021

Choose a reason for hiding this comment

i-norden Feb 24, 2021 • edited Loading

Choose a reason for hiding this comment

i-norden Feb 24, 2021

Choose a reason for hiding this comment

amaury1093 Feb 25, 2021

Choose a reason for hiding this comment

i-norden Feb 25, 2021

Choose a reason for hiding this comment

amaury1093 left a comment

Choose a reason for hiding this comment

i-norden commented Feb 25, 2021

aaronc commented Mar 30, 2021

i-norden commented Mar 30, 2021

i-norden commented Feb 9, 2021 •

edited

Loading

codecov bot commented Feb 9, 2021 •

edited

Loading

i-norden Feb 24, 2021 •

edited

Loading