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.

Sign up for GitHub

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

Jump to bottom

Query timeout #874

Closed
wants to merge 3 commits into from
Closed

Query timeout #874

wants to merge 3 commits into from

Conversation

Mallets
Copy link
Member

@Mallets Mallets commented Mar 29, 2024

This PR adds a tombstone for query timeout to actually disable any timeout and any associated task spawning.
In practice, it is now possible to configure no timeout at all in both queries and infrastructure.

@Mallets Mallets added the enhancement Existing things could work better label Mar 29, 2024
@eclipse-zenoh-bot
Copy link
Contributor

@Mallets If this pull request contains a bugfix or a new feature, then please consider using Closes #ISSUE-NUMBER syntax to link it to an issue.

@Mallets Mallets requested a review from JEnoch March 29, 2024 11:27
@eclipse-zenoh-bot
Copy link
Contributor

@Mallets If this pull request contains a bugfix or a new feature, then please consider using Closes #ISSUE-NUMBER syntax to link it to an issue.

@Mallets Mallets mentioned this pull request Mar 29, 2024
Closed
@Mallets
Copy link
Member Author

Mallets commented Mar 29, 2024

@JEnoch I'm not very fond of using u64::MAX (i.e., 18446744073709551615) to disable timeout. A better approach may be to use 0? This of course will require some change on rmw_zenoh. What do you think?

@JEnoch
Copy link
Member

JEnoch commented Mar 29, 2024

That makes sense. I don't think that's an issue to change this in rmw_zenoh at this stage.

However I think the comment in DEFAULT_CONFIG.json5 (and therefore in rmw_zenoh's default config files) should be more explicit wrt. the relationship and/or difference between the timeout in config and the timeout in Query API.
And similarly in the doc for Query timeout API.

@DenisBiryukov91 DenisBiryukov91 mentioned this pull request Mar 29, 2024
Merged
@Mallets
Copy link
Member Author

Mallets commented Apr 11, 2024

Superseded by #881

@Mallets Mallets closed this Apr 11, 2024
@Mallets Mallets deleted the query_timeout branch April 11, 2024 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JEnoch JEnoch Awaiting requested review from JEnoch

Assignees
No one assigned
Labels
enhancement Existing things could work better
Projects
Zenoh 1.0.0 release
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Mallets @eclipse-zenoh-bot @JEnoch