Skip to content

Add ability to specify wait time to QueueInputStream#452

Merged
garydgregory merged 2 commits intoapache:masterfrom
maxxedev:QueueInputStream-wait-timeout
Apr 22, 2023
Merged

Add ability to specify wait time to QueueInputStream#452
garydgregory merged 2 commits intoapache:masterfrom
maxxedev:QueueInputStream-wait-timeout

Conversation

@maxxedev
Copy link
Contributor

  • Revert the change that adds PollingQueueInputStream and TakingQueueInputStream
  • Add ability to specify wait time to QueueInputStream

I think this will keep the API simple yet offer the roughly the same (or even more) functionality the reverted change.
- PollingQueueInputStream is same as new QueueInputStream() which has timeout=0
- TakingQueueInputStream is same as new QueueInputStream(..., Duration.ofMillis(Long.MAX_VALUE))

and users can also specify arbitrary queue read timeout between 0 and LONG_MAX

see also https://github.com/apache/commons-io/pull/447

@garydgregory garydgregory merged commit 150ba4a into apache:master Apr 22, 2023
@garydgregory
Copy link
Member

@maxxedev
TY for the PR.

I merged it and updated the code to use a builder instead of adding yet another constructor (see the rest of the code base).

What feels odd is that LinkedBlockingQueue polls using nanoseconds and we now use milliseconds as the internal timeout, so we cannot use the most precise polling. Any thoughts on changing milliseconds to nanoseconds?

@maxxedev
Copy link
Contributor Author

Thanks for quick review and merge.

Good suggestion about nanos. I did not notice that LinkedBlockingQueue polls using nanoseconds. Here is the PR: https://github.com/apache/commons-io/pull/453

@maxxedev
Copy link
Contributor Author

Is a changes.xml entry needed?

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