Restructure FNS global cache to be list #2036

dydxwill · 2024-08-05T20:31:06Z

Changelist

Restructure FNS global cache to be list

Test Plan

Tested in dev2

Author/Reviewer Checklist

If this PR has changes that result in a different app state given the same prior state and transaction list, manually add the state-breaking label.
If the PR has breaking postgres changes to the indexer add the indexer-postgres-breaking label.
If this PR isn't state-breaking but has changes that modify behavior in PrepareProposal or ProcessProposal, manually add the label proposal-breaking.
If this PR is one of many that implement a specific feature, manually label them all feature:[feature-name].
If you wish to for mergify-bot to automatically create a PR to backport your change to a release branch, manually add the label backport/[branch-name].
Manually add any of the following labels: refactor, chore, bug.

Summary by CodeRabbit

New Features
- Improved management of stream updates and subscriptions, enhancing performance and simplifying update handling.
- Added branch trigger for the GitHub Actions workflow to allow builds on the wl/rm_global_cache branch.
Bug Fixes
- Resolved issues related to subscription ID management, ensuring accurate associations with stream updates.
Refactor
- Streamlined the GitHub Actions workflow by consolidating multiple build and push jobs into a single job for improved maintainability.

coderabbitai · 2024-08-05T20:31:14Z

Walkthrough

Recent updates to the FullNodeStreamingManagerImpl class and the GitHub Actions workflow enhance stream update management and streamline CI/CD processes. The transition from map-based structures to lists for stream updates improves subscription handling, while consolidating multiple CI jobs into a single workflow simplifies maintenance and enhances clarity.

Changes

Files	Change Summary
`protocol/streaming/full_node_streaming_manager.go`	- Restructured `FullNodeStreamingManagerImpl` to use lists for stream updates and introduced a new mapping for subscriptions. - Updated the constructor and methods to align with these changes.
`.github/workflows/protocol-build-and-push.yml`	- Simplified the CI/CD workflow by removing jobs for various environments, retaining only the `build-and-push-dev5` job for streamlined management. - Added a trigger for the `wl/rm_global_cache` branch to enhance automation.

Poem

In the meadow, streams do flow,
With updates swift, like rabbits go!
Subscriptions dance on lists so bright,
Hopping along, a joyful sight!
Performance boosted, logic clear,
A warren of code, we now hold dear! 🐇✨

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 340ea7c and 3c7a63b.

Files selected for processing (1)

protocol/streaming/full_node_streaming_manager.go (9 hunks)

Additional comments not posted (6)

protocol/streaming/full_node_streaming_manager.go (6)

72-77: LGTM! Ensure new data structures are correctly initialized.

The new list-based caches are correctly initialized.

142-153: LGTM! Ensure mapping is correctly populated.

The new mapping clobPairIdToSubscriptionIdMapping is correctly populated.

315-336: LGTM! Ensure updates are correctly grouped and sent.

The updates are correctly grouped by clob pair IDs and sent using the new list-based structure.

Line range hint 354-379:
LGTM! Ensure fills are correctly grouped and sent.

The fills are correctly grouped by clob pair IDs and sent using the new list-based structure.

Line range hint 383-397:
LGTM! Ensure updates and subscription IDs are correctly added to the cache.

The updates and subscription IDs are correctly added to the cache using the new list-based structure.

Line range hint 420-459:
LGTM! Ensure updates and subscription IDs are correctly flushed.

The updates and subscription IDs are correctly flushed using the new list-based structure.

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 3c7a63b and af79f20.

Files selected for processing (1)

.github/workflows/protocol-build-and-push.yml (2 hunks)

Files skipped from review due to trivial changes (1)

.github/workflows/protocol-build-and-push.yml

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between af79f20 and 57e54fa.

Files selected for processing (1)

.github/workflows/protocol-build-and-push.yml (3 hunks)

Files skipped from review as they are similar to previous changes (1)

.github/workflows/protocol-build-and-push.yml

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 57e54fa and e6fcbf5.

Files selected for processing (1)

protocol/streaming/full_node_streaming_manager.go (9 hunks)

Additional comments not posted (6)

protocol/streaming/full_node_streaming_manager.go (6)

72-77: Initialization of new data structures looks good.

The constructor correctly initializes the new list-based data structures.

142-152: Mapping population looks correct.

The clobPairIdToSubscriptionIdMapping is correctly populated with subscription IDs.

314-335: Correct usage of list-based structure for updates.

The updates are correctly grouped and added to the cache using the new list-based structure.

Line range hint 353-378:
Correct usage of list-based structure for fill updates.

The fill updates are correctly grouped and added to the cache using the new list-based structure.

Line range hint 382-399:
Correct addition of updates and subscription IDs to cache.

The updates and subscription IDs are correctly added to the cache using the new list-based structure.

Line range hint 422-461:
Correct handling of updates with new list-based structure.

The updates are correctly collected and sent to subscribers using the new list-based structure.

jonfung-dydx · 2024-08-06T18:02:45Z

protocol/streaming/full_node_streaming_manager.go

+	// list of subscription ids for each stream update.
+	streamUpdateSubscriptionCache     [][]uint32
+	clobPairIdToSubscriptionIdMapping map[uint32][]uint32
+	numUpdatesInCache                 uint32


prob no need for this field now that we have a central list, can use len(list)

jonfung-dydx · 2024-08-06T18:03:51Z

protocol/streaming/full_node_streaming_manager.go

+	streamUpdateCache []clobtypes.StreamUpdate
+	// list of subscription ids for each stream update.
+	streamUpdateSubscriptionCache     [][]uint32
+	clobPairIdToSubscriptionIdMapping map[uint32][]uint32


comment for these 2 new fields?

jonfung-dydx · 2024-08-06T18:05:43Z

protocol/streaming/full_node_streaming_manager.go

+		}
+		sm.clobPairIdToSubscriptionIdMapping[clobPairId] = append(
+			sm.clobPairIdToSubscriptionIdMapping[clobPairId],
+			sm.nextSubscriptionId,


do we evict stuff from this map when we prune order subscriptions from the fns manager?

done, thanks for the catch!

jonfung-dydx · 2024-08-06T18:08:15Z

protocol/streaming/full_node_streaming_manager.go

@@ -295,27 +311,28 @@ func (sm *FullNodeStreamingManagerImpl) SendOrderbookUpdates(
 	}

 	// Unmarshal each per-clob pair message to v1 updates.
-	updatesByClobPairId := make(map[uint32][]clobtypes.StreamUpdate)
+	updatesByClobPairId := make([]clobtypes.StreamUpdate, 0)


rename var since this is not a map anymore?

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e6fcbf5 and 8d333b8.

Files selected for processing (1)

protocol/streaming/full_node_streaming_manager.go (11 hunks)

Files skipped from review as they are similar to previous changes (1)

protocol/streaming/full_node_streaming_manager.go

jayy04 · 2024-08-06T19:38:57Z

protocol/streaming/full_node_streaming_manager.go

+	streamUpdateCache []clobtypes.StreamUpdate
+	// list of subscription ids for each stream update.
+	streamUpdateSubscriptionCache [][]uint32


nit: cleaner to combine these two since they are used together and assumed to have same length etc

added a TODO for now, will address in followup PR

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 8d333b8 and 40a26d5.

Files selected for processing (1)

.github/workflows/protocol-build-and-push.yml (1 hunks)

Files skipped from review as they are similar to previous changes (1)

.github/workflows/protocol-build-and-push.yml

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 40a26d5 and c238336.

Files selected for processing (1)

protocol/streaming/full_node_streaming_manager.go (11 hunks)

Files skipped from review as they are similar to previous changes (1)

protocol/streaming/full_node_streaming_manager.go

rm global cache

3c7a63b

github-actions bot added the protocol label Aug 5, 2024

coderabbitai bot reviewed Aug 5, 2024

View reviewed changes

dev5

af79f20

coderabbitai bot reviewed Aug 5, 2024

View reviewed changes

dydxwill added 2 commits August 5, 2024 16:48

add branch

96cb022

dev2

57e54fa

coderabbitai bot reviewed Aug 6, 2024

View reviewed changes

dydxwill added 2 commits August 6, 2024 12:14

lint

ce15619

revert ghwf

e6fcbf5

coderabbitai bot reviewed Aug 6, 2024

View reviewed changes

jonfung-dydx reviewed Aug 6, 2024

View reviewed changes

address cmts

8d333b8

coderabbitai bot reviewed Aug 6, 2024

View reviewed changes

dydxwill requested a review from jonfung-dydx August 6, 2024 18:26

jonfung-dydx approved these changes Aug 6, 2024

View reviewed changes

jayy04 reviewed Aug 6, 2024

View reviewed changes

jayy04 approved these changes Aug 6, 2024

View reviewed changes

gh wf

40a26d5

coderabbitai bot reviewed Aug 6, 2024

View reviewed changes

dydxwill added 2 commits August 7, 2024 10:18

add todo

3e2325f

revert ghwf

c238336

coderabbitai bot reviewed Aug 7, 2024

View reviewed changes

dydxwill merged commit 1b0f052 into main Aug 7, 2024
17 checks passed

dydxwill deleted the wl/rm_global_cache branch August 7, 2024 14:37

jonfung-dydx pushed a commit that referenced this pull request Aug 15, 2024

Restructure FNS global cache to be list (#2036)

73d39d3

jonfung-dydx pushed a commit that referenced this pull request Sep 17, 2024

Restructure FNS global cache to be list (#2036)

611e058

jonfung-dydx pushed a commit that referenced this pull request Sep 26, 2024

Restructure FNS global cache to be list (#2036)

f6694dd

coderabbitai bot mentioned this pull request Sep 26, 2024

Add a cli method for vault withdrawal #2369

Merged

6 tasks

jonfung-dydx pushed a commit that referenced this pull request Oct 4, 2024

Restructure FNS global cache to be list (#2036)

7e8ef15

jonfung-dydx pushed a commit that referenced this pull request Oct 18, 2024

Restructure FNS global cache to be list (#2036)

3b06a02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restructure FNS global cache to be list #2036

Restructure FNS global cache to be list #2036

dydxwill commented Aug 5, 2024 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Aug 5, 2024 •

edited

Loading

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

coderabbitai bot left a comment

coderabbitai bot left a comment

coderabbitai bot left a comment

coderabbitai bot left a comment

jonfung-dydx Aug 6, 2024

dydxwill Aug 6, 2024

jonfung-dydx Aug 6, 2024

dydxwill Aug 6, 2024

jonfung-dydx Aug 6, 2024

dydxwill Aug 6, 2024 •

edited

Loading

jonfung-dydx Aug 6, 2024

dydxwill Aug 6, 2024

coderabbitai bot left a comment

jayy04 Aug 6, 2024

dydxwill Aug 7, 2024

coderabbitai bot left a comment

coderabbitai bot left a comment

Restructure FNS global cache to be list #2036

Restructure FNS global cache to be list #2036

Conversation

dydxwill commented Aug 5, 2024 • edited by coderabbitai bot Loading

Changelist

Test Plan

Author/Reviewer Checklist

Summary by CodeRabbit

coderabbitai bot commented Aug 5, 2024 • edited Loading

Walkthrough

Changes

Poem

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dydxwill Aug 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

dydxwill commented Aug 5, 2024 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Aug 5, 2024 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

dydxwill Aug 6, 2024 •

edited

Loading