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

Add protocol extensions for user attachment #590

Merged
merged 23 commits into from
Dec 14, 2023
Merged

Add protocol extensions for user attachment #590

merged 23 commits into from
Dec 14, 2023

Conversation

Mallets
Copy link
Member

@Mallets Mallets commented Nov 15, 2023

This PR adds an attachment extension to Put, Del, Query, and Reply Zenoh messages.
This is a necessary step to allow the user to attach any arbitrary metadata to put, del, query, and reply operations.

An example of some on-going effort of such API in zenoh-c can be found here: eclipse-zenoh/zenoh-c#190

Although I believe this PR will not evolve any further, I'll keep it in Draft for the time being since a validation with a full attachment API is desirable before merging.

In addition, this PR explicitly deconstructs any type in the write codec in such a way that, anytime a new protocol extension is added, the compiler will highlight where the new extension needs to be handled in the code.

@Mallets Mallets marked this pull request as ready for review December 14, 2023 10:06

let data: Value = (0usize..size)
let data: Value = (0usize..dbg!(payload_size))
Copy link
Member Author

Choose a reason for hiding this comment

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

Why dbg!?

Copy link
Contributor

Choose a reason for hiding this comment

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

forgot to remove it, done now

@Mallets
Copy link
Member Author

Mallets commented Dec 14, 2023

This PR introduces a throughput regression of ~200K msg/s. However, #610 improved throughput of ~200K msg/s. So, we are even. Nevertheless, @p-avital I think it's worth investigating why this PR introduces such a throughput drop.

Approving this PR so as to unblock eclipse-zenoh/zenoh-c#203 and eclipse-zenoh/zenoh-cpp#86.

@p-avital p-avital self-requested a review December 14, 2023 14:20
@Mallets Mallets requested a review from p-avital December 14, 2023 14:21
@Mallets Mallets merged commit 49011f1 into master Dec 14, 2023
14 of 15 checks passed
@Mallets Mallets deleted the attachment branch December 14, 2023 14:33
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.

2 participants