refactor!: ADR-038 go-plugin system #14207

egaxhaj · 2022-12-07T18:39:08Z

Description

replaces #11691

implements #13473

This is an extension and refactor of the existing ADR-038 streaming service work to introduce a plugin system over gRPC to the SDK and load streaming services using the hashicorp/go-plugin system rather the approach taken in #11691 (which uses Go's built in Plugins API).

The plugin system introduced here is meant to be extensible, so that if other components/features of the SDK wish to be included as plugins in the future they can write plugins with minimal effort by defining an interface and a gRPC message protocol and leveraging the built in plugin system.

Closes: #XXXX

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

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)

and refactoring file streamer to output single file for each block update spec sort storeKey once at first move MemoryListener store package

tac0turtle · 2023-03-14T09:21:36Z

@egaxhaj could you update to master and then we can merge this pr.

tac0turtle · 2023-03-14T09:21:58Z

What is in the 22MB file store/streaming/abci/examples/file/file, should this be committed?

@egaxhaj would be good to get more information on this as well.

kocubinski · 2023-03-14T13:16:18Z

Personally I don't think we should be adding so many replace directives to go.mod files if a go.work file would do that job. Does CI fail without the replace directive?

I also don't understand why we're committing 44 MB of opaque binary files.

tac0turtle · 2023-03-14T13:20:42Z

Does CI fail without the replace directive?

Replace directives is something we do now or we run go get on the version in the pr. This removes the need for the replace tag. Then once tagged dependabot makes the change. We dont commit the go.work file, it is mainly meant for local development. this has caused some confusion

egaxhaj · 2023-03-14T15:22:48Z

What is in the 22MB file store/streaming/abci/examples/file/file, should this be committed?

@egaxhaj would be good to get more information on this as well.

These are the go example plugin binaries (stdout, file). I can remove the file binary but the stdout binary is required to run the unit tests during CI as it is the default.

tac0turtle · 2023-03-14T15:35:30Z

What is in the 22MB file store/streaming/abci/examples/file/file, should this be committed?

@egaxhaj would be good to get more information on this as well.

These are the go example plugin binaries (stdout, file). I can remove the file binary but the stdout binary is required to run the unit tests during CI as it is the default.

lets remove the file and the binary may be harder.

kocubinski · 2023-03-14T16:46:54Z

Replace directives is something we do now or we run go get on the version in the pr. This removes the need for the replace tag.

What is a "replace tag"?

We dont commit the go.work file, it is mainly meant for local development. this has caused some confusion

I know what we don't, and also understand the function of a go.work is for local development. That's why I asked if CI failed if the replace directives are removed. So, does it?

tac0turtle · 2023-03-14T17:01:45Z

Replace directives is something we do now or we run go get on the version in the pr. This removes the need for the replace tag.

What is a "replace tag"?

https://go.dev/ref/mod#go-mod-file-replace same thing as replace directive

We dont commit the go.work file, it is mainly meant for local development. this has caused some confusion

I know what we don't, and also understand the function of a go.work is for local development. That's why I asked if CI failed if the replace directives are removed. So, does it?

On other prs it can, im not sure about this pr.

egaxhaj · 2023-03-14T17:09:26Z

What is in the 22MB file store/streaming/abci/examples/file/file, should this be committed?

@egaxhaj would be good to get more information on this as well.

These are the go example plugin binaries (stdout, file). I can remove the file binary but the stdout binary is required to run the unit tests during CI as it is the default.

lets remove the file and the binary may be harder.

I've removed the file plugin binary and added a .gitignore and readme. Conflicts have been resolved as well.

egaxhaj · 2023-03-14T17:39:39Z

@tac0turtle can we coordinate the merge for this PR? It's hard to keep up with other merges to main. I've had to update the branch 3 times in the last hour.

egaxhaj · 2023-03-14T17:49:15Z

Replace directives is something we do now or we run go get on the version in the pr. This removes the need for the replace tag.

What is a "replace tag"?

https://go.dev/ref/mod#go-mod-file-replace same thing as replace directive

We dont commit the go.work file, it is mainly meant for local development. this has caused some confusion

I know what we don't, and also understand the function of a go.work is for local development. That's why I asked if CI failed if the replace directives are removed. So, does it?

On other prs it can, im not sure about this pr.

I am certain that some tests in CI will fail on this PR. Running make test locally fails with compilation errors if the replace tags are not in place because the other modules don't currently have the plugin code that resides in the store & baseapp package.

julienrbrt · 2023-03-14T18:33:55Z

🚀
@egaxhaj do you wanna do the chore (removing the replaces and using a pseudo version from main) as a follow-up or should someone from our team do it?

egaxhaj · 2023-03-14T18:47:12Z

Someone from the team please. I'm not familiar with the process the team follows.

julienrbrt · 2023-03-14T20:58:19Z

baseapp/abci.go

+	for _, abciListener := range app.streamingManager.ABCIListeners {
+		ctx := app.deliverState.ctx
+		blockHeight := ctx.BlockHeight()
+		if err := abciListener.ListenBeginBlock(app.deliverState.ctx, req, res); err != nil {


Curious, doing the chore, and here, why app.deliverState.ctx is used and not ctx?

Oversight. You can go ahead and use ctx.

Co-authored-by: HuangYi <[email protected]> Co-authored-by: Ryan Christoffersen <[email protected]>

Ergels Gaxhaj and others added 30 commits September 23, 2022 12:28

use hashicorp go-plugin

3f57c9d

python example

1a6db28

implement streaming to baseapp

3db64a0

abci listener wiring

041d83a

update test files

0bf1b54

update test files

1e4914e

kafka plugin python example

02fa1db

update package name

1c5ce62

update proto and services

13f750c

tx and kvpair indexing

0c4c1bb

tidy up

040bfa0

add readme

7691fc2

update readme

9ccf4fe

add testing section

db7840f

listeners need to be async

78e697b

remove goroutine on blocking call

c0871cd

terminate with os.Exit on stopNodeOnErr

56c643d

dumb pipe bridge

fcd438b

fix: listen state writes on rootmulti store only

460638a

and refactoring file streamer to output single file for each block update spec sort storeKey once at first move MemoryListener store package

introduce memory listener and OnCommit listener

a022370

remove WriteListener and use MemoryListener

dcc7915

internalize listener init to cache layer

ef0a861

determenistic PopStateCache, grpc service

6a72ea4

add Go file plugin

a67756e

multiple service support

ac5fa45

add \n

c17b5ad

add proto dir

3edd320

append to file

55196f4

support in-process listeners

e6c29ce

use setStreamingService

d1e2ffa

do not commit file plugin binary

27f68c0

Merge branch 'main' into egaxhaj/adr-038-grpc2

a6d5f88

Merge branch 'main' into egaxhaj/adr-038-grpc2

45bd990

Merge branch 'main' into egaxhaj/adr-038-grpc2

4f94ae0

Merge branch 'main' into egaxhaj/adr-038-grpc2

96e0854

tac0turtle enabled auto-merge (squash) March 14, 2023 17:57

tac0turtle merged commit 6f3f2c9 into cosmos:main Mar 14, 2023

julienrbrt mentioned this pull request Mar 14, 2023

build(deps): Bump github.com/hashicorp/go-plugin from 1.4.8 to 1.4.9 in /store #15399

Closed

julienrbrt mentioned this pull request Mar 14, 2023

chore: clean-up after go-plugin system pr #15402

Merged

19 tasks

julienrbrt reviewed Mar 14, 2023

View reviewed changes

egaxhaj deleted the egaxhaj/adr-038-grpc2 branch March 14, 2023 22:22

faddat mentioned this pull request Mar 28, 2023

update linter to 1521 ChihuahuaChain/cosmos-sdk#6

Closed

19 tasks

larry0x pushed a commit to larry0x/cosmos-sdk that referenced this pull request May 22, 2023

refactor!: ADR-038 go-plugin system (cosmos#14207)

421ed1e

Co-authored-by: HuangYi <[email protected]> Co-authored-by: Ryan Christoffersen <[email protected]>

This was referenced Jun 29, 2023

fix(baseapp): audit changes #16596

Merged

Audit baseapp v0.47...v0.50 #16474

Closed

julienrbrt mentioned this pull request Aug 2, 2023

refactor: reword upgrading.md and verify match with repo #17262

Merged

20 tasks

julienrbrt mentioned this pull request Nov 16, 2023

fix: pass FinalizeBlock request and response to ABCIListener #18486

Merged

20 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor!: ADR-038 go-plugin system #14207

refactor!: ADR-038 go-plugin system #14207

egaxhaj commented Dec 7, 2022

tac0turtle commented Mar 14, 2023

tac0turtle commented Mar 14, 2023

kocubinski commented Mar 14, 2023

tac0turtle commented Mar 14, 2023 •

edited

Loading

egaxhaj commented Mar 14, 2023

tac0turtle commented Mar 14, 2023

kocubinski commented Mar 14, 2023 •

edited

Loading

tac0turtle commented Mar 14, 2023 •

edited

Loading

egaxhaj commented Mar 14, 2023

egaxhaj commented Mar 14, 2023

egaxhaj commented Mar 14, 2023

julienrbrt commented Mar 14, 2023

egaxhaj commented Mar 14, 2023

julienrbrt Mar 14, 2023

egaxhaj Mar 14, 2023 •

edited

Loading

refactor!: ADR-038 go-plugin system #14207

refactor!: ADR-038 go-plugin system #14207

Conversation

egaxhaj commented Dec 7, 2022

Description

Author Checklist

Reviewers Checklist

tac0turtle commented Mar 14, 2023

tac0turtle commented Mar 14, 2023

kocubinski commented Mar 14, 2023

tac0turtle commented Mar 14, 2023 • edited Loading

egaxhaj commented Mar 14, 2023

tac0turtle commented Mar 14, 2023

kocubinski commented Mar 14, 2023 • edited Loading

tac0turtle commented Mar 14, 2023 • edited Loading

egaxhaj commented Mar 14, 2023

egaxhaj commented Mar 14, 2023

egaxhaj commented Mar 14, 2023

julienrbrt commented Mar 14, 2023

egaxhaj commented Mar 14, 2023

julienrbrt Mar 14, 2023

Choose a reason for hiding this comment

egaxhaj Mar 14, 2023 • edited Loading

Choose a reason for hiding this comment

tac0turtle commented Mar 14, 2023 •

edited

Loading

kocubinski commented Mar 14, 2023 •

edited

Loading

tac0turtle commented Mar 14, 2023 •

edited

Loading

egaxhaj Mar 14, 2023 •

edited

Loading