Re: [Discussion] Race on heartbeat update in checkpoint thread

2021-08-18 Thread Andrey Gura
Alexey, Ilya, I took a look at the problem and corresponding fixes. It seems that you are both right. But Alexey's fix looks like some kind of hack to me. We have two problems: 1. Heartbeat update from thread that will complete a future. I agree with Ilya and Andrey. Only a critical worker itse

Re: [Discussion] Race on heartbeat update in checkpoint thread

2021-07-28 Thread Ilya Kazakov
Guys. I have discussed just these changes. Look at my PR, please. What do you think? https://issues.apache.org/jira/browse/IGNITE-15192 https://github.com/apache/ignite/pull/9280/files чт, 22 июл. 2021 г. в 22:30, Andrey Mashenkov : > Hi guys, > > I think updateHeartBeat() method was misused in

Re: [Discussion] Race on heartbeat update in checkpoint thread

2021-07-22 Thread Andrey Mashenkov
Hi guys, I think updateHeartBeat() method was misused in the future listener and this must be fixed. Actually, GridWorker.heartbeatTs Javadoc says that field is updated by the worker itself. It is consistent with WorkProgressDispatcher.updateHearbeat() javadoc, which said "Notifying dispatcher th

Re: [Discussion] Race on heartbeat update in checkpoint thread

2021-07-21 Thread Alex Plehanov
Ilya, Race only affects the future listener (which only updates heartbeat), but not checkpoint listeners, so no other bugs possible caused by this race except wrong heartbeat update. As I've written before, I think it's not a good idea to wait for other threads by the critical thread in the block

Re: [Discussion] Race on heartbeat update in checkpoint thread

2021-07-20 Thread Ilya Kazakov
1. I mean calling listeners in CheckpointWorkflow.markCheckpointBegin(): This -- for (CheckpointListener lsnr : dbLsnrs) lsnr.beforeCheckpointBegin(ctx0); ctx0.awaitPendingTasksFinished(); -- And this: -

Re: [Discussion] Race on heartbeat update in checkpoint thread

2021-07-20 Thread Ivan Daschinsky
+1 For current fix. Code is clean and understandable. I suppose that the current fix is a correct variant to update heartbeatTs. вт, 20 июл. 2021 г. в 16:13, Alex Plehanov : > Hello, Ilya > > > But anyway, I propose to remove the update of the heartbeat from other > threads altogether and wrap th

Re: [Discussion] Race on heartbeat update in checkpoint thread

2021-07-20 Thread Alex Plehanov
Hello, Ilya > But anyway, I propose to remove the update of the heartbeat from other threads altogether and wrap the call to listeners in a blockingSection. I don't quite understand your proposal. Which call to listeners do you mean? If we wrap the listener into the blocking section the result wil

[Discussion] Race on heartbeat update in checkpoint thread

2021-07-20 Thread Ilya Kazakov
Hi Igniters, hi Alexey. I want to discuss this issue: https://issues.apache.org/jira/browse/IGNITE-15099. I have caught it too. I was able to determine where there is a race. The update of the heartbeat happens asynchronously into the listener code. But we always wait in the checkpoint thread fo