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.

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

[cleanup][indexer] Clean up unused protos and add new proto for grpc #5761

Merged
merged 4 commits into from
Dec 4, 2022

Conversation

larry-aptos
Copy link
Contributor

@larry-aptos larry-aptos commented Dec 3, 2022

Description

  • Cleaning up the streamingfast unused protos.
  • Adding/building a new proto for our upcoming work on indexer.

Test Plan

@larry-aptos larry-aptos marked this pull request as ready for review December 3, 2022 01:40
@bowenyang007 bowenyang007 changed the title [Datastream][protos] add the Datastream service proto v1. [cleanup] add the Datastream service proto v1. Dec 3, 2022
@bowenyang007 bowenyang007 changed the title [cleanup] add the Datastream service proto v1. [cleanup] Clean up unused protos and add new proto for grpc Dec 3, 2022
@bowenyang007 bowenyang007 changed the title [cleanup] Clean up unused protos and add new proto for grpc [cleanup][indexer] Clean up unused protos and add new proto for grpc Dec 3, 2022
@bowenyang007 bowenyang007 enabled auto-merge (squash) December 3, 2022 02:01
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2022

✅ Forge suite land_blocking success on 09697d0ab6f4e2aa23d0da23b3d6497cdf0b57fd

performance benchmark with full nodes : 6613 TPS, 5996 ms latency, 13200 ms p99 latency,(!) expired 160 out of 2824220 txns
Test Ok

@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2022

✅ Forge suite compat success on testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 09697d0ab6f4e2aa23d0da23b3d6497cdf0b57fd

Compatibility test results for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 09697d0ab6f4e2aa23d0da23b3d6497cdf0b57fd (PR)
1. Check liveness of validators at old version: testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b
compatibility::simple-validator-upgrade::liveness-check : 7252 TPS, 5288 ms latency, 8200 ms p99 latency,no expired txns
2. Upgrading first Validator to new version: 09697d0ab6f4e2aa23d0da23b3d6497cdf0b57fd
compatibility::simple-validator-upgrade::single-validator-upgrade : 4281 TPS, 9544 ms latency, 12700 ms p99 latency,no expired txns
3. Upgrading rest of first batch to new version: 09697d0ab6f4e2aa23d0da23b3d6497cdf0b57fd
compatibility::simple-validator-upgrade::half-validator-upgrade : 4919 TPS, 8449 ms latency, 11100 ms p99 latency,no expired txns
4. upgrading second batch to new version: 09697d0ab6f4e2aa23d0da23b3d6497cdf0b57fd
compatibility::simple-validator-upgrade::rest-validator-upgrade : 6979 TPS, 5486 ms latency, 10300 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 09697d0ab6f4e2aa23d0da23b3d6497cdf0b57fd passed
Test Ok

@bowenyang007 bowenyang007 merged commit 2a23246 into main Dec 4, 2022
@bowenyang007 bowenyang007 deleted the datastrema-service-protobuf branch December 4, 2022 22:47
@Markuze Markuze mentioned this pull request Dec 5, 2022
@ppoliani
Copy link
Contributor

ppoliani commented Dec 5, 2022

I can see this PR removes the extractor.proto. However, this is what we currently use in our Streaming fast indexer which utilises Firehose. Is there a documentation explaining the reasons behind this so we can determine what future holds for our indexer?

@larry-aptos
Copy link
Contributor Author

I can see this PR removes the extractor.proto. However, this is what we currently use in our Streaming fast indexer which utilises Firehose. Is there a documentation explaining the reasons behind this so we can determine what future holds for our indexer?

We'll add extractor.proto back soon. @bowenyang007 is working on this.

@bowenyang007
Copy link
Contributor

bowenyang007 commented Dec 6, 2022

@ppoliani We removed that piece of code b/c it's unmaintained (both firehose-stream and extractor.proto). We are building a grpc endpoint so I'm sort of adding it back, but we make no guarantee that the schema will be the same b/c we are not integrated with StreamingFast's Firehose service.

If you want to maintain the integration the best way is to create your own fork.

@ppoliani
Copy link
Contributor

ppoliani commented Dec 6, 2022

@bowenyang007 thank you for the clarification. Does it mean that this config will also become obsolete?

I'm currently running this firehose-enabled node . Should I expect that future versions of the Aptos node will not work with that code?

areshand pushed a commit to areshand/aptos-core-1 that referenced this pull request Dec 18, 2022
…ptos-labs#5761)

* add the Datastream proto v1.

* add the Datastream proto v1.
@Markuze Markuze mentioned this pull request Dec 26, 2022
@YaroShkvorets
Copy link

So is extractor.proto coming back?

@bowenyang007
Copy link
Contributor

So is extractor.proto coming back?

Unfortunately it's not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants