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

Adds header to bifrost payloads #1520

Merged
merged 3 commits into from
May 21, 2024
Merged

Adds header to bifrost payloads #1520

merged 3 commits into from
May 21, 2024

Conversation

AhmedSoliman
Copy link
Contributor

@AhmedSoliman AhmedSoliman commented May 16, 2024

Adds header to bifrost payloads

This also changes the loglet API to accept Bytes to avoid duplication of serialization across loglet implementations. This holds the current invariant that payloads are opaque to loglets.


Stack created with Sapling. Best reviewed with ReviewStack.

@AhmedSoliman AhmedSoliman changed the title Add header to bifrost payloads Adds header to bifrost payloads May 16, 2024
@AhmedSoliman AhmedSoliman force-pushed the pr1520 branch 2 times, most recently from 3223c89 to 8af59b4 Compare May 16, 2024 09:04
Copy link

github-actions bot commented May 16, 2024

Test Results

 99 files  ±0   99 suites  ±0   8m 0s ⏱️ -39s
 84 tests ±0   82 ✅ ±0  2 💤 ±0  0 ❌ ±0 
216 runs  ±0  210 ✅ ±0  6 💤 ±0  0 ❌ ±0 

Results for commit 7e01a6a. ± Comparison against base commit bb0645f.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@tillrohrmann tillrohrmann left a comment

Choose a reason for hiding this comment

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

Headers for bifrost payloads :-) LGTM. +1 for merging.

created_at: MillisSinceEpoch,
// additional custom headers can be added here. Those should be somewhat
// generic and values must be optional.
pub custom_data_1: Option<u64>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this more of an example of how to add new fields or is custom_data_1 already used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, it's used by my perf. I might come back to this and give it a meaningful name, but I thought to keep it agnostic.

@AhmedSoliman AhmedSoliman force-pushed the pr1520 branch 2 times, most recently from 1bac118 to 8f8a640 Compare May 21, 2024 09:33
This PR includes:
- Attachment of PPM to cluster controller now use Networking
- PPM observes partition processor status through a buffered watch mechanism
- PPM can now send control messages (unused at this PR) to processor for future use.
- PPM collects state information from running processors. This will be used in a follow PR to response to controller requests about partitions.


Cluster controller grpc service is kept for external tooling integration (CLI, etc.)
ClusterState is a new data structure maintained by ClusterController. ClusterState is established by a scatter-gather request (GetProcessorsState) to all workers. For every worker, partition processor manager is the entity responsible for responding to GetProcessorsState requests, it keeps taps on running partition processors via a set of tokio watches.

This PR is a snapshot in time of progress done on this component, this is far from complete but it's best to merge the current progress until we circle back to it in a week or two.
This also changes the loglet API to accept Bytes to avoid duplication of serialization across loglet implementations. This holds the current invariant that payloads are opaque to loglets.
@AhmedSoliman AhmedSoliman merged commit 45fd2a9 into main May 21, 2024
10 checks passed
@AhmedSoliman AhmedSoliman deleted the pr1520 branch May 21, 2024 09:48
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.

None yet

2 participants