On 11/17/2015 9:37 PM, Yakov Zhdanov wrote:
1. Understood
2. Well, I understand that object gets deserialized on get from cache and
this most probably should not cause any issue. However, code does not look
healthy and someone may think that there is an issue - field is not
volatile, initialized in constructor, has sync setter, but is accessed
several times in code outside of sync. Can you please refactor this?
3. Yes, Denis is looking into this. And I think your issue will be fixed in
the scope of Denis one.
I'm afraid that my changes won't fix the issue in Vladislav's
implementation.
However, when my changes are merged we will be able to rework these
semaphore failover tests and fix any issue in the semaphore impl related
to failover. I can take care of this.
--
Denis
Vlad, can you please refactor [2] and I will pick it up and merge tomorrow.
Thank you for your interest and contribution!
--Yakov
2015-11-17 21:30 GMT+04:00 Vladisav Jelisavcic <vladis...@gmail.com>:
Hi Yakov,
1. Yes
2. if you mean that nodeMap is accessed in onNodeRemoved(UUID nodeID)
method of the GridCacheSemaphoreImpl class,
it shouldn't be a problem, but it can be changed easily not to do so;
3.
org.apache.ignite.internal.processors.cache.datastructures.GridCacheAbstractDataStructuresFailoverSelfTest#testSemaphoreConstantTopologyChangeFailoverSafe()
org.apache.ignite.internal.processors.cache.datastructures.GridCacheAbstractDataStructuresFailoverSelfTest#testSemaphoreConstantMultipleTopologyChangeFailoverSafe()
I think the problem is with the atomicity of the simulated grid failure;
once stopGrid() is called for a node, other threads on this same node start
throwing interrupted exceptions,
which are in turn not handled properly in the
GridCacheAbstractDataStructuresFailoverSelfTest;
Those exceptions shouldn't be dealt with inside the GridCacheSemaphoreImpl
itself.
In a realworld node failure scenario, all those threads would fail at the
same time
(none of them would influence the rest of the grid anymore);
I think fixing the issue Denis is working on can fix this (IGNITE-801 and
IGNITE-803)
Am i right? Does it makes sense?
Best regards,
Vladisav
