This is an automated email from the ASF dual-hosted git repository.

yong pushed a commit to branch branch-4.15
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git

commit 0336c3b35e31ec7600b22a3443c4316ffe7e677e
Author: Aditya Kumbhar <[email protected]>
AuthorDate: Sun Nov 13 19:24:16 2022 -0600

    Use LinkedHashMap instead of HashMap to fix flaky tests due 
non-deterministic order (#3551)
    
    ### Motivation
    
    AuditorReplicasCheckTest has the following tests that are flaky, detected 
using the [NonDex](https://github.com/TestingResearchIllinois/NonDex) tool.
    testReplicasCheckForLedgersFoundHavingLessThanAQReplicasOfAnEntry()
    testReplicasCheckForLedgersFoundHavingLessThanWQReplicasOfAnEntry()
    testReplicasCheckForLedgersFoundHavingNoReplica()
    testReplicasCheckForLedgersWithEmptySegments()
    
    The tests fail due to non-deterministic ordering in the HashMap 
`segmentEnsembles`
    The following check fails in the `newEnsembleEntry()` method since it 
assumes the ordering of `segmentEnsembles` to be deterministic:
    ```
    checkArgument(ensembles.isEmpty() || firstEntry > ensembles.lastKey(),
                          "New entry must have a first entry greater than any 
existing ensemble key");
    ```
    Fixes #3550
    
    Command to reproduce:
    ```
    mvn -pl bookkeeper-server edu.illinois:nondex-maven-plugin:1.1.2:nondex 
-Dtest=org.apache.bookkeeper.replication.AuditorReplicasCheckTest
    ```
    
    ### Changes
    
    Changed the datatype of `segmentEnsembles` from HashMap to 
[LinkedHashMap](https://docs.oracle.com/javase/8/docs/api/java/util/LinkedHashMap.html)
 as it has a deterministic iteration order, while 
[HashMap](https://docs.oracle.com/javase/8/docs/api/java/util/HashMap.html) 
does not guarantee the order of its elements.
    The tests pass with NonDex after making this change.
    
    (cherry picked from commit f8b9e1886d1f6a806150e450cc525933f6c9063f)
---
 .../bookkeeper/replication/AuditorReplicasCheckTest.java     | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git 
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AuditorReplicasCheckTest.java
 
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AuditorReplicasCheckTest.java
index dbd6c806f1..00cb2e82d4 100644
--- 
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AuditorReplicasCheckTest.java
+++ 
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AuditorReplicasCheckTest.java
@@ -27,7 +27,7 @@ import java.net.URI;
 import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CompletableFuture;
@@ -300,7 +300,7 @@ public class AuditorReplicasCheckTest extends 
BookKeeperClusterTestCase {
          * BookieHandleNotAvailableException so asyncGetListOfEntriesOfLedger 
will
          * return BookieHandleNotAvailableException.
          */
-        Map<Long, List<BookieId>> segmentEnsembles = new HashMap<Long, 
List<BookieId>>();
+        Map<Long, List<BookieId>> segmentEnsembles = new LinkedHashMap<Long, 
List<BookieId>>();
         segmentEnsembles.put(0L, bookieAddresses);
         long ledgerId = 1L;
         createClosedLedgerMetadata(lm, ledgerId, ensembleSize, 
writeQuorumSize, ackQuorumSize, segmentEnsembles,
@@ -398,7 +398,7 @@ public class AuditorReplicasCheckTest extends 
BookKeeperClusterTestCase {
          * Empty one for all of the bookies, so this ledger would be counted in
          * ledgersFoundHavingNoReplicaOfAnEntry .
          */
-        Map<Long, List<BookieId>> segmentEnsembles = new HashMap<Long, 
List<BookieId>>();
+        Map<Long, List<BookieId>> segmentEnsembles = new LinkedHashMap<Long, 
List<BookieId>>();
         segmentEnsembles.put(0L, bookieAddresses);
         long ledgerId = 1L;
         createClosedLedgerMetadata(lm, ledgerId, ensembleSize, 
writeQuorumSize, ackQuorumSize, segmentEnsembles,
@@ -528,7 +528,7 @@ public class AuditorReplicasCheckTest extends 
BookKeeperClusterTestCase {
          * would be counted towards
          * ledgersFoundHavingLessThanAQReplicasOfAnEntry.
          */
-        Map<Long, List<BookieId>> segmentEnsembles = new HashMap<Long, 
List<BookieId>>();
+        Map<Long, List<BookieId>> segmentEnsembles = new LinkedHashMap<Long, 
List<BookieId>>();
         int ensembleSize = 4;
         int writeQuorumSize = 3;
         int ackQuorumSize = 2;
@@ -681,7 +681,7 @@ public class AuditorReplicasCheckTest extends 
BookKeeperClusterTestCase {
          * for this ledger a copy of entry 3, so this ledger would be counted
          * towards ledgersFoundHavingLessThanWQReplicasOfAnEntry.
          */
-        Map<Long, List<BookieId>> segmentEnsembles = new HashMap<Long, 
List<BookieId>>();
+        Map<Long, List<BookieId>> segmentEnsembles = new LinkedHashMap<Long, 
List<BookieId>>();
         int ensembleSize = 4;
         int writeQuorumSize = 3;
         int ackQuorumSize = 2;
@@ -836,7 +836,7 @@ public class AuditorReplicasCheckTest extends 
BookKeeperClusterTestCase {
          * numLedgersFoundHavingNoReplicaOfAnEntry/LessThanAQReplicasOfAnEntry
          * /WQReplicasOfAnEntry.
          */
-        Map<Long, List<BookieId>> segmentEnsembles = new HashMap<Long, 
List<BookieId>>();
+        Map<Long, List<BookieId>> segmentEnsembles = new LinkedHashMap<Long, 
List<BookieId>>();
         int ensembleSize = 4;
         int writeQuorumSize = 3;
         int ackQuorumSize = 2;

Reply via email to