sergey-chugunov-1985 commented on code in PR #13101:
URL: https://github.com/apache/ignite/pull/13101#discussion_r3196339597
##########
modules/core/src/main/java/org/apache/ignite/internal/cluster/IgniteKillTask.java:
##########
@@ -69,25 +72,19 @@ class IgniteKillTask extends ComputeTaskAdapter<Boolean,
Void> {
* Kill job.
*/
private class IgniteKillJob extends ComputeJobAdapter {
+ /** */
+ @IgniteInstanceResource
+ private Ignite ignite;
Review Comment:
There is a comment from code analisys tool saying that we need to make
Ignite field transient. I think it is a good idea.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/diagnostic/pagelocktracker/SharedPageLockTracker.java:
##########
@@ -320,17 +329,38 @@ public void stop() {
/**
*
*/
- private class TimeOutWorker extends CycleThread {
+ private class TimeOutWorker extends GridWorker {
+ /** Sleep interval before each iteration. */
+ private final long sleepInterval;
/**
+ * Creates new timeout worker with given parameters.
*
+ * @param sleepInterval sleep interval before each iteration
*/
- TimeOutWorker(long interval) {
- super("page-lock-tracker-timeout", interval);
+ protected TimeOutWorker(String igniteInstanceName, long sleepInterval,
IgniteLogger log) {
+ super(igniteInstanceName, "page-lock-tracker-timeout", log);
+
+ this.sleepInterval = sleepInterval;
}
/** {@inheritDoc} */
- @Override public void iteration() {
+ @SuppressWarnings("BusyWait")
+ @Override public final void body() {
+ try {
+ while (!runner().isInterrupted()) {
+ Thread.sleep(sleepInterval);
+
+ iteration();
+ }
+ }
+ catch (InterruptedException e) {
Review Comment:
We should not catch this exception. Otherwise I see a risk of losing an
interruption signal and stucking in the while loop forever (as
`runner().isInterrupted()` will be returning `false` because we caught
InterruptedException but didn't restore interrupted state of the runner thread).
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/GridUpdateNotifier.java:
##########
@@ -103,23 +104,21 @@ class GridUpdateNotifier {
this.updatesChecker = updatesChecker;
this.reportOnlyNew = reportOnlyNew;
- workerThread = new Thread(new Runnable() {
- @Override public void run() {
- try {
- while (!Thread.currentThread().isInterrupted()) {
- Runnable cmd0 = cmd.getAndSet(null);
+ workerThread = new IgniteThread(igniteInstanceName, "upd-ver-checker",
() -> {
+ try {
+ while (!Thread.currentThread().isInterrupted()) {
+ Runnable cmd0 = cmd.getAndSet(null);
- if (cmd0 != null)
- cmd0.run();
- else
- Thread.sleep(WORKER_THREAD_SLEEP_TIME);
- }
- }
- catch (InterruptedException ignore) {
- // No-op.
+ if (cmd0 != null)
+ cmd0.run();
+ else
+ Thread.sleep(WORKER_THREAD_SLEEP_TIME);
}
}
- }, "upd-ver-checker");
+ catch (InterruptedException ignore) {
Review Comment:
We should not catch InterruptedException here as well.
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/diagnostic/pagelocktracker/SharedPageLockTrackerTest.java:
##########
@@ -305,11 +304,16 @@ private void doTestTakeDumpByTime(
public void testMemoryLeakOnThreadTerminates() throws Exception {
int threadLimits = 1000;
int timeOutWorkerInterval = 10_000;
- Consumer<Set<PageLockThreadState>> hnd = (threads) -> {
- };
+ Consumer<Set<PageLockThreadState>> hnd = (threads) -> {};
Review Comment:
Please address SonarCube comment
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
