Adds resume functionality #33
Conversation
Problem statement: - Previously, there was no resume() function that would restart the consumer after cancellation. Proposed changes: - Implements resume() function - Adds CANCELLING_BUT_RESUMING state to the consumer - CANCELLING_BUT_RESUMING state is added to support prefetch update in the future
examples/resume/README.md
Outdated
| @@ -0,0 +1,3 @@ | |||
| New state CANCELLING_BUT_RESUMING is introduced to support resume after cancellation | |||
|
|
|||
|  | |||
There was a problem hiding this comment.
Let's put this image in this repo under the docs folder, and add a brief markdown file in there which references it. This README describes internals to the library but so far the examples folder is used for end-users to quickly get started. I think we should keep those purposes separate
| if (d_state == NOT_CONSUMING) { | ||
| d_state = CANCELLED; | ||
| return method; | ||
| } |
There was a problem hiding this comment.
Highlighting this change for the PR history (Shynar already discussed this internally). This fixes an issue where calling cancel on a consumer while the consumer isn't actually consuming, for example due to a connection issue, can put it in a stuck state.
|
The Doxygen CI step seems to be trying to write to this repo, which is (rightly) failing for a PR coming from a fork. I'm OK to ignore this CI check for now and someone can address the problem with this check separately |
mvrsss
force-pushed
the
resume_consumer
branch
from
2a7e213
to
98b3ed9
Compare
|
|
||
| - previously cancelled consumer would be restarted after `resume()` call | ||
|
|
||
|  |
There was a problem hiding this comment.
|  | |
|  |
Need to commit the image into the git repo, but we could make that a separate PR to facilitate merging this now
Problem statement:
Proposed changes: