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

Support receiveAsync cancellation #298

Closed
wants to merge 1 commit into from
Closed

Support receiveAsync cancellation #298

wants to merge 1 commit into from

Conversation

sschepens
Copy link
Contributor

We should allow for futures returned by receiveAsync to be cancelled and remove the future from pendingReceives queue.

@sschepens sschepens force-pushed the receive_async_cancellation branch from 03a8aad to f969b3b Compare March 20, 2017 21:31
@merlimat
Copy link
Contributor

@sschepens The change looks good to me. Can you expand on what would be the use case for cancelling the receive future? Also can you add a unit test for the new behavior?

@sschepens
Copy link
Contributor Author

@merlimat We're building a process that consumes from multiple consumers and then triggers http requests. We're using netty and building it in an async way.
The best way to consume messages in an async way is to use receiveAsync(), other options are synchronous and would require for us to use some scheduling logic when no messages are received to not block threads.
Now, we would also like to be able to have a timeout on receiveAsync or be able to cancel it if we wish to control the amount of receiveAsync calls that are pending at any time.

I will add a test, just wanted to check with you first

@sschepens sschepens force-pushed the receive_async_cancellation branch from f969b3b to 6fefab9 Compare April 3, 2017 19:21
@sschepens sschepens force-pushed the receive_async_cancellation branch from 6fefab9 to 0eaa78b Compare April 24, 2017 16:25
@merlimat merlimat modified the milestone: 1.19 Jun 14, 2017
hrsakai pushed a commit to hrsakai/pulsar that referenced this pull request Dec 10, 2020
@merlimat
Added semaphore implementation with lower contention (apache#298)
cc08fd6
hangc0276 pushed a commit to hangc0276/pulsar that referenced this pull request May 26, 2021
@BewareMyPower
Add kstream aggregation tests (apache#298)
b4508c7 
This PR Ports KStreamAggregationIntegrationTest from Kafka.

Besides, from this PR, KoP only tests Kafka Streams tests with entryFormat=kafka since the correctness of entryFormat has already been verified by tests under io.streamnative.pulsar.handlers.kop.

* Add KStreamAggregationTest

* Test Kafka Streams only for one entry format
@xiaotongwang1 xiaotongwang1 mentioned this pull request Aug 4, 2021
Closed
@dave2wave
Copy link
Member

This PR is evidently stale or abandoned. Reopen if this is not so.

@dave2wave dave2wave closed this Dec 16, 2021
@github-actions
Copy link

@sschepens:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

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

@sschepens sschepens

Labels
doc-label-missing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sschepens @merlimat @dave2wave