Fixed a data loss issue on lite member promotion #17644

petrpleshachkov · 2020-09-30T09:23:15Z

Notify cluster members on lite member promotion to update
memberGroupSize. Otherwise, a data member might be not aware of other
promoted data members, and it may cause backup operations
not being issued to other data members.

Closes #17621

Notify cluster members on lite member promotion to update memberGroupSize. Otherwise, a data member might be not aware of other promoted data members, and it may cause backup operations not being issued to other data members. Closes hazelcast#17621

mdogan · 2020-09-30T09:41:06Z

hazelcast/src/main/java/com/hazelcast/internal/cluster/impl/MembershipManager.java

@@ -367,6 +367,7 @@ private MemberImpl createNewMemberImplIfChanged(MemberInfo newMemberInfo, Member
                member = clusterService.promoteAndGetLocalMember();
            } else {
                member = createMember(newMemberInfo, member.getAttributes());
+                node.partitionService.memberAdded(member);


Shouldn't this be out of else block? What if this is on local member?

On the local member the node.partitionService.memberAdded(member); is doing update only when the node is not local. For local case, it issues a special task only when the node is a master. But the master is notified about promotion without this change. This is why I decided that it is enough to put it in the else.

Also, the problem is when the member is not aware of other members being promoted. That case is covered in the else branch

I think issue is a non-master member is not aware of any member being promoted, including itself. So maybe we should modify node.partitionService.memberAdded(member) to update `memberGroupSize even with local member.

See this modified test:

@org.junit.Test public void lite_member_promotion_data_loss() throws InterruptedException { int entryCount = 1000; TestHazelcastInstanceFactory factory = new TestHazelcastInstanceFactory(); Config config = new Config().setLiteMember(true); // start first hazelcast instance as a lite member HazelcastInstance firstHazelcastInstance = factory.newHazelcastInstance(config); // start second hazelcast instance as a lite member HazelcastInstance secondHazelcastInstance = factory.newHazelcastInstance(config); // promote all instances to data members firstHazelcastInstance.getCluster().promoteLocalLiteMember(); secondHazelcastInstance.getCluster().promoteLocalLiteMember(); // check if cluster is in a good shape assertTrueEventually(() -> assertTrue(firstHazelcastInstance.getPartitionService().isClusterSafe())); // insert some dummy data into the testing map String mapName = randomMapName(); IMap<String, String> testMap = firstHazelcastInstance.getMap(mapName); for (int i = 0; i < entryCount; ++i) { testMap.put("key" + i, "value" + i); } // check all data is correctly inserted assertEquals(entryCount, testMap.size()); // kill second instance secondHazelcastInstance.getLifecycleService().terminate(); // backup count for the map is set to 1 // even with 1 node down, no data loss is expected assertTrueEventually(() -> assertEquals(entryCount, firstHazelcastInstance.getMap(mapName).size())); }

hazelcast/src/main/java/com/hazelcast/internal/cluster/impl/MembershipManager.java

ahmetmircik · 2020-10-05T16:39:25Z

hazelcast/src/test/java/com/hazelcast/internal/cluster/impl/PromoteLiteMemberTest.java

+    }
+
+    @Test
+    public void lite_member_promotion_data_loss2() throws InterruptedException {


ahmetmircik · 2020-10-05T16:39:27Z

hazelcast/src/test/java/com/hazelcast/internal/cluster/impl/PromoteLiteMemberTest.java

@@ -323,6 +326,86 @@ public void promotion_shouldFail_whenMasterIsSuspected_duringPromotion() throws
        }
    }

+    @Test
+    public void lite_member_promotion_data_loss() throws InterruptedException {


can we give a better name to test? Seeing the state tested against and expected behavior in test method name would be helpful for the future readers.

ahmetmircik · 2020-10-05T17:20:44Z

hazelcast/src/main/java/com/hazelcast/internal/cluster/impl/MembershipManager.java

@@ -342,6 +346,10 @@ void updateMembers(MembersView membersView) {

        setMembers(MemberMap.createNew(membersView.getVersion(), members));

+        if (updatedLiteMember) {
+            updateMembersGroupSize();


it is hard to to follow similar named methods maybe we can make updateMemberGroupsSize() method public and can call it here: node.partitionService.getPartitionStateManager().updateMemberGroupsSize()

ahmetmircik · 2020-10-05T17:24:01Z

hazelcast/src/main/java/com/hazelcast/internal/cluster/impl/MembershipManager.java

        for (MemberInfo memberInfo : membersView.getMembers()) {
            Address address = memberInfo.getAddress();
            MemberImpl member = currentMemberMap.getMember(address);

            if (member != null && member.getUuid().equals(memberInfo.getUuid())) {
+                if (member.isLiteMember()) {
+                    updatedLiteMember = true;


can you please add a short comment to the code to explain the meaning of updatedLiteMember?

mdogan · 2020-10-08T06:58:16Z

@petrpleshachkov: This fix should be backported 4.0.z and 3.12.z branches.

petrpleshachkov · 2020-10-08T10:49:31Z

@ahmetmircik , I've addressed the comments.

petrpleshachkov · 2020-10-09T07:35:00Z

Guys, thanks for the review, I'm merging the PR.

mmedenjak · 2020-10-14T19:30:38Z

@petrpleshachkov have you backported the PR?

Notify cluster members on lite member promotion to update memberGroupSize. Otherwise, a data member might be not aware of other promoted data members, and it may cause backup operations not being issued to other data members. Closes hazelcast#17621

Fixed a data loss issue on lite member promotion (#17644) Notify cluster members on lite member promotion to update memberGroupSize. Otherwise, a data member might be not aware of other promoted data members, and it may cause backup operations not being issued to other data members. Closes #17621

…17758) Notify cluster members on lite member promotion to update memberGroupSize. Otherwise, a data member might be not aware of other promoted data members, and it may cause backup operations not being issued to other data members. Closes #17621

petrpleshachkov added Type: Defect Team: Core Priority: Medium Source: Internal PR or issue was opened by an employee Estimation: S Module: Cluster labels Sep 30, 2020

petrpleshachkov added this to the 4.1 milestone Sep 30, 2020

petrpleshachkov requested a review from mdogan September 30, 2020 09:23

petrpleshachkov self-assigned this Sep 30, 2020

mdogan reviewed Sep 30, 2020

View reviewed changes

Update memberGroupSize on every lite membership change

336f250

mdogan reviewed Oct 5, 2020

View reviewed changes

hazelcast/src/main/java/com/hazelcast/internal/cluster/impl/MembershipManager.java Outdated Show resolved Hide resolved

Removed unnecessary code

a050910

petrpleshachkov requested a review from ahmetmircik October 5, 2020 11:19

mdogan approved these changes Oct 5, 2020

View reviewed changes

ahmetmircik reviewed Oct 5, 2020

View reviewed changes

Addressed reviewer's comment

7ac4040

ahmetmircik approved these changes Oct 8, 2020

View reviewed changes

petrpleshachkov merged commit 82bd5e7 into hazelcast:master Oct 9, 2020

mmedenjak mentioned this pull request Dec 1, 2020

[BACKPORT] Fixed a data loss issue on lite member promotion (#17644) #17758

Merged

mmedenjak mentioned this pull request Dec 1, 2020

Fixed a data loss issue on lite member promotion (#17644) #17757

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed a data loss issue on lite member promotion #17644

Fixed a data loss issue on lite member promotion #17644

petrpleshachkov commented Sep 30, 2020

mdogan Sep 30, 2020

petrpleshachkov Sep 30, 2020

petrpleshachkov Sep 30, 2020

mdogan Sep 30, 2020 •

edited

ahmetmircik Oct 5, 2020

ahmetmircik Oct 5, 2020

ahmetmircik Oct 5, 2020

ahmetmircik Oct 5, 2020

mdogan commented Oct 8, 2020

petrpleshachkov commented Oct 8, 2020

petrpleshachkov commented Oct 9, 2020

mmedenjak commented Oct 14, 2020

Fixed a data loss issue on lite member promotion #17644

Fixed a data loss issue on lite member promotion #17644

Conversation

petrpleshachkov commented Sep 30, 2020

mdogan Sep 30, 2020

Choose a reason for hiding this comment

petrpleshachkov Sep 30, 2020

Choose a reason for hiding this comment

petrpleshachkov Sep 30, 2020

Choose a reason for hiding this comment

mdogan Sep 30, 2020 • edited

Choose a reason for hiding this comment

ahmetmircik Oct 5, 2020

Choose a reason for hiding this comment

ahmetmircik Oct 5, 2020

Choose a reason for hiding this comment

ahmetmircik Oct 5, 2020

Choose a reason for hiding this comment

ahmetmircik Oct 5, 2020

Choose a reason for hiding this comment

mdogan commented Oct 8, 2020

petrpleshachkov commented Oct 8, 2020

petrpleshachkov commented Oct 9, 2020

mmedenjak commented Oct 14, 2020

mdogan Sep 30, 2020 •

edited