Skip to content

Added read/write timeout support to Rust TTcpChannel#3399

Open
ShemShadrack wants to merge 1 commit intoapache:masterfrom
ShemShadrack:thrift
Open

Added read/write timeout support to Rust TTcpChannel#3399
ShemShadrack wants to merge 1 commit intoapache:masterfrom
ShemShadrack:thrift

Conversation

@ShemShadrack
Copy link
Copy Markdown

Summary

Add timeout configuration support to the Rust TTcpChannel.

This change adds:

  • set_read_timeout
  • set_write_timeout
  • set_timeouts

Timeouts are applied immediately when the channel already wraps an open
TcpStream, and cached for later application when the channel is opened
after configuration.

Testing

Verified with:

  • cargo fmt --manifest-path lib/rs/Cargo.toml --all -- --check
  • cargo test --manifest-path lib/rs/Cargo.toml

Added unit tests covering:

  • setting timeouts before open()
  • applying cached timeouts during open()
  • setting timeouts on wrapped streams
  • clearing timeouts

@Jens-G
Copy link
Copy Markdown
Member

Jens-G commented Apr 19, 2026

Can we have a JIRA ticket for this one pls?

Comment thread lib/rs/test/Cargo.toml
log = "0.4"
uuid = "1"
uuid = ">=1, <1.21"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do we want to do that? If there is an incompatibility with >= 1.21, the proper approach would be to fix the incompatibility.

-1 I'm against it. Even more as the change is not explained at all.

stream: Some(stream),
read_timeout: None,
write_timeout: None,
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The incoming stream may have timeouts configured. This gives an inconsistent state.


/// Set the read and write timeouts for this channel.
pub fn set_timeouts(
&mut self,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we also have getters to make it complete?

@Jens-G
Copy link
Copy Markdown
Member

Jens-G commented Apr 19, 2026

BTW We still look for Rust maintainers as the previous one got lost somehow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants