IGNITE-28681 Operation Context integrated in Ignite internal async structures#13148
IGNITE-28681 Operation Context integrated in Ignite internal async structures#13148petrov-mg wants to merge 12 commits into
Conversation
petrov-mg
force-pushed
the
IGNITE-28681
branch
4 times, most recently
from
00e2378 to
4d73864
Compare
|
|
||
| /** */ | ||
| protected IgniteThread createWorkerThread(GridWorker worker) { | ||
| return new IgniteThread(igniteInstanceName(), name(), worker, GRP_IDX_UNASSIGNED, -1, GridIoPolicy.UNDEFINED); |
There was a problem hiding this comment.
We could use another IgniteThread constructor with only three arguments. grpIdx, stripe and plc parameters are always the same.
| } | ||
|
|
||
| /** {@inheritDoc} */ | ||
| @Override protected void cleanup() { |
There was a problem hiding this comment.
I guess cleanup method is called as part of node's stop procedure and race with start method is not possible?
By race I mean a situation when the first thread calls start method, the second on calls cleanup and then the third calls start again and reverts the work of the second thread.
If this situation is not possible it is fine to ignore this comment.
There was a problem hiding this comment.
The start method creates and starts a worker thread. Once the worker thread completes execution of its body, it invokes the cleanup method (see GridWorker#run). Since cleanup is declared as protected, it can only be invoked by the worker thread itself.
Therefore, invocation of start always happens-before invocation of cleanup.
Currently, AsyncQueueHandler supports restarting, so the following scenario is expected:
- A worker thread is started.
- The worker is cancelled.
- The worker body completes execution.
- Resources are cleaned up.
- The worker thread terminates.
- A new worker thread is started.
We expect that code that uses AsyncQueueHandler must prevent unintended worker restarts.
| } | ||
|
|
||
| /** */ | ||
| public void scheduleAutoFlush(DataStreamerImpl<?, ?> dataStreamer) { |
There was a problem hiding this comment.
Why two methods exposed through GridKernalContext instead of passing flusher to DataStreamerImpl? Both approaches are not ideal but the last one is less invasive from my point of view.
| /** | ||
| * Cancel thread execution and completes all notification futures. | ||
| */ | ||
| /** Cancel thread execution and completes all notification futures. */ |
There was a problem hiding this comment.
| /** Cancel thread execution and completes all notification futures. */ | |
| /** Cancels thread execution and completes all notification futures. */ |
|
2 participants
Thank you for submitting the pull request to the Apache Ignite.
In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:
The Contribution Checklist
The description explains WHAT and WHY was made instead of HOW.
The following pattern must be used:
IGNITE-XXXX Change summarywhereXXXX- number of JIRA issue.(see the Maintainers list)
the
green visaattached to the JIRA ticket (see tabPR Checkat TC.Bot - Instance 1 or TC.Bot - Instance 2)Notes
If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com #ignite channel.