You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2018/08/28 09:37:27 UTC
[GitHub] stefan-egli closed pull request #1: SLING-7830 : introduce invertLeaderElectionPrefixOrder and leaderElec…
stefan-egli closed pull request #1: SLING-7830 : introduce invertLeaderElectionPrefixOrder and leaderElec…
URL: https://github.com/apache/sling-org-apache-sling-discovery-oak/pull/1
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/src/main/java/org/apache/sling/discovery/oak/Config.java b/src/main/java/org/apache/sling/discovery/oak/Config.java
index 4bb4095..e47cd9d 100644
--- a/src/main/java/org/apache/sling/discovery/oak/Config.java
+++ b/src/main/java/org/apache/sling/discovery/oak/Config.java
@@ -179,6 +179,16 @@
private static final String BACKOFF_STABLE_FACTOR = "backoffStableFactor";
private static final int DEFAULT_BACKOFF_STABLE_FACTOR = 5;
+ private static final long DEFAULT_LEADER_ELECTION_PREFIX = 1;
+ @Property(longValue=DEFAULT_LEADER_ELECTION_PREFIX)
+ private static final String LEADER_ELECTION_PREFIX = "leaderElectionPrefix";
+ protected long leaderElectionPrefix = DEFAULT_LEADER_ELECTION_PREFIX;
+
+ private static final boolean DEFAULT_INVERT_LEADER_ELECTION_PREFIX_ORDER = false;
+ @Property(boolValue=DEFAULT_INVERT_LEADER_ELECTION_PREFIX_ORDER)
+ private static final String INVERT_LEADER_ELECTION_PREFIX_ORDER = "invertLeaderElectionPrefixOrder";
+ protected boolean invertLeaderElectionPrefixOrder = DEFAULT_INVERT_LEADER_ELECTION_PREFIX_ORDER;
+
/** True when auto-stop of a local-loop is enabled. Default is false. **/
private boolean autoStopLocalLoopEnabled;
@@ -339,6 +349,17 @@ protected void configure(final Map<String, Object> properties) {
DEFAULT_BACKOFF_STANDBY_FACTOR);
backoffStableFactor = PropertiesUtil.toInteger(properties.get(BACKOFF_STABLE_FACTOR),
DEFAULT_BACKOFF_STABLE_FACTOR);
+
+ this.invertLeaderElectionPrefixOrder = PropertiesUtil.toBoolean(
+ properties.get(INVERT_LEADER_ELECTION_PREFIX_ORDER),
+ DEFAULT_INVERT_LEADER_ELECTION_PREFIX_ORDER);
+ logger.debug("configure: invertLeaderElectionPrefixOrder='{}'",
+ this.invertLeaderElectionPrefixOrder);
+ this.leaderElectionPrefix = PropertiesUtil.toLong(
+ properties.get(LEADER_ELECTION_PREFIX),
+ DEFAULT_LEADER_ELECTION_PREFIX);
+ logger.debug("configure: leaderElectionPrefix='{}'",
+ this.leaderElectionPrefix);
}
/**
@@ -506,4 +527,12 @@ public long getClusterSyncServiceIntervalMillis() {
public boolean getSyncTokenEnabled() {
return syncTokenEnabled;
}
+
+ public boolean isInvertLeaderElectionPrefixOrder() {
+ return invertLeaderElectionPrefixOrder;
+ }
+
+ public long getLeaderElectionPrefix() {
+ return leaderElectionPrefix;
+ }
}
diff --git a/src/main/java/org/apache/sling/discovery/oak/cluster/OakClusterViewService.java b/src/main/java/org/apache/sling/discovery/oak/cluster/OakClusterViewService.java
index af30f20..2632806 100644
--- a/src/main/java/org/apache/sling/discovery/oak/cluster/OakClusterViewService.java
+++ b/src/main/java/org/apache/sling/discovery/oak/cluster/OakClusterViewService.java
@@ -196,14 +196,7 @@ private LocalClusterView asClusterView(DiscoveryLiteDescriptor descriptor, Resou
leaderElectionIds.put(id, leaderElectionId);
}
- Collections.sort(activeIdsList, new Comparator<Integer>() {
-
- @Override
- public int compare(Integer arg0, Integer arg1) {
- return leaderElectionIds.get(arg0)
- .compareTo(leaderElectionIds.get(arg1));
- }
- });
+ leaderElectionSort(activeIdsList, leaderElectionIds);
for(int i=0; i<activeIdsList.size(); i++) {
int id = activeIdsList.get(i);
@@ -233,6 +226,66 @@ public int compare(Integer arg0, Integer arg1) {
}
}
+ private void leaderElectionSort(List<Integer> activeIdsList, final Map<Integer, String> leaderElectionIds) {
+ final Comparator<Integer> comparator;
+ if (config.isInvertLeaderElectionPrefixOrder()) {
+ // SLING-7830 : inverted leaderElectionPrefix sorting
+ comparator = new Comparator<Integer>() {
+
+ private long prefixOf(String leaderElectionId) {
+ final int underScore = leaderElectionId.indexOf("_");
+ if (underScore == -1) {
+ return -1;
+ }
+ final String prefixStr = leaderElectionId.substring(0, underScore);
+ try{
+ return Long.parseLong(prefixStr);
+ } catch(Exception e) {
+ return -1;
+ }
+ }
+
+ @Override
+ public int compare(Integer arg0, Integer arg1) {
+ // 'inverted sorting' means that the prefix is ordered descending
+ // while the remainder is ordered ascending
+ final String leaderElectionId0 = leaderElectionIds.get(arg0);
+ final String leaderElectionId1 = leaderElectionIds.get(arg1);
+ // so first step is to order the part before '_', eg the '1' in
+ // 1_0000001534409616936_374019fc-68bd-4c8d-a4cf-8ee8b07c63bc
+ final long prefix0 = prefixOf(leaderElectionId0);
+ final long prefix1 = prefixOf(leaderElectionId1);
+ // if a prefix is -1 (due to eg wrong formatting) it automatically
+ // ends up at the end
+ if (prefix0 == prefix1) {
+ // if they are the same, order the classic way
+ // note that when they are both '-1' that can be one of the following
+ // -1_0000001534409616936_374019fc-68bd-4c8d-a4cf-8ee8b07c63bc
+ // _0000001534409616936_374019fc-68bd-4c8d-a4cf-8ee8b07c63bc
+ // notALong_0000001534409616936_374019fc-68bd-4c8d-a4cf-8ee8b07c63bc
+ // so all of the above three get compared the classic way
+ return leaderElectionId0
+ .compareTo(leaderElectionId1);
+ } else {
+ // inverted order comparison:
+ return new Long(prefix1).compareTo(prefix0);
+ }
+ }
+
+ };
+ } else {
+ comparator = new Comparator<Integer>() {
+
+ @Override
+ public int compare(Integer arg0, Integer arg1) {
+ return leaderElectionIds.get(arg0)
+ .compareTo(leaderElectionIds.get(arg1));
+ }
+ };
+ }
+ Collections.sort(activeIdsList, comparator);
+ }
+
/**
* oak's discovery-lite can opt to not provide a clusterViewId eg in the
* single-VM case. (for clusters discovery-lite normally defines the
diff --git a/src/main/java/org/apache/sling/discovery/oak/pinger/OakViewChecker.java b/src/main/java/org/apache/sling/discovery/oak/pinger/OakViewChecker.java
index 6938d13..3773166 100644
--- a/src/main/java/org/apache/sling/discovery/oak/pinger/OakViewChecker.java
+++ b/src/main/java/org/apache/sling/discovery/oak/pinger/OakViewChecker.java
@@ -292,7 +292,7 @@ private String newLeaderElectionId() {
String currentTimeMillisStr = String.format("%0"
+ maxLongLength + "d", System.currentTimeMillis());
- String prefix = "1";
+ String prefix = String.valueOf(config.getLeaderElectionPrefix());
final String newLeaderElectionId = prefix + "_"
+ currentTimeMillisStr + "_" + slingId;
diff --git a/src/test/java/org/apache/sling/discovery/oak/OakDiscoveryServiceTest.java b/src/test/java/org/apache/sling/discovery/oak/OakDiscoveryServiceTest.java
index 4a41e7f..cfff2cc 100644
--- a/src/test/java/org/apache/sling/discovery/oak/OakDiscoveryServiceTest.java
+++ b/src/test/java/org/apache/sling/discovery/oak/OakDiscoveryServiceTest.java
@@ -19,6 +19,7 @@
package org.apache.sling.discovery.oak;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
@@ -37,6 +38,7 @@
import org.apache.sling.discovery.base.its.setup.VirtualInstance;
import org.apache.sling.discovery.base.its.setup.mock.DummyResourceResolverFactory;
import org.apache.sling.discovery.base.its.setup.mock.MockFactory;
+import org.apache.sling.discovery.commons.providers.BaseTopologyView;
import org.apache.sling.discovery.commons.providers.base.DummyListener;
import org.apache.sling.discovery.commons.providers.spi.base.DescriptorHelper;
import org.apache.sling.discovery.commons.providers.spi.base.DiscoveryLiteConfig;
@@ -333,4 +335,53 @@ private DiscoveryLiteDescriptorBuilder prefill(DiscoveryLiteDescriptor d) throws
private ResourceResolver getResourceResolver(ResourceResolverFactory resourceResolverFactory) throws LoginException {
return resourceResolverFactory.getServiceResourceResolver(null);
}
+
+ @Test
+ public void testInvertedLeaderElectionOrder() throws Exception {
+ logger.info("testInvertedLeaderElectionOrder: start");
+ // instance1 should be leader
+ BaseTopologyView lastView = doTestInvertedLeaderElectionOrder(20, 10);
+ assertTrue(lastView.getLocalInstance().isLeader());
+ // instance2 should be leader
+ lastView = doTestInvertedLeaderElectionOrder(10, 20);
+ assertFalse(lastView.getLocalInstance().isLeader());
+ // instance1 should be leader
+ lastView = doTestInvertedLeaderElectionOrder(10, 10);
+ assertTrue(lastView.getLocalInstance().isLeader());
+ }
+
+ private BaseTopologyView doTestInvertedLeaderElectionOrder(final int instance1Prefix, final int instance2Prefix)
+ throws Exception {
+ OakVirtualInstanceBuilder builder1 =
+ (OakVirtualInstanceBuilder) new OakVirtualInstanceBuilder()
+ .setDebugName("instance1")
+ .newRepository("/foo/barrio/foo/", true)
+ .setConnectorPingInterval(999)
+ .setConnectorPingTimeout(999);
+ builder1.getConfig().leaderElectionPrefix = instance1Prefix;
+ builder1.getConfig().invertLeaderElectionPrefixOrder = true;
+ VirtualInstance instance1 = builder1.build();
+ OakVirtualInstanceBuilder builder2 =
+ (OakVirtualInstanceBuilder) new OakVirtualInstanceBuilder()
+ .setDebugName("instance2")
+ .useRepositoryOf(instance1)
+ .setConnectorPingInterval(999)
+ .setConnectorPingTimeout(999);
+ builder2.getConfig().leaderElectionPrefix = instance2Prefix;
+ builder2.getConfig().invertLeaderElectionPrefixOrder = true;
+ VirtualInstance instance2 = builder2.build();
+
+ DummyListener listener = new DummyListener();
+ OakDiscoveryService discoveryService = (OakDiscoveryService) instance1.getDiscoveryService();
+ discoveryService.bindTopologyEventListener(listener);
+
+ instance1.heartbeatsAndCheckView();
+ instance2.heartbeatsAndCheckView();
+ instance1.heartbeatsAndCheckView();
+ instance2.heartbeatsAndCheckView();
+
+ assertEquals(0, discoveryService.getViewStateManager().waitForAsyncEvents(2000));
+ assertEquals(1, listener.countEvents());
+ return listener.getLastView();
+ }
}
diff --git a/src/test/java/org/apache/sling/discovery/oak/its/setup/OakVirtualInstanceBuilder.java b/src/test/java/org/apache/sling/discovery/oak/its/setup/OakVirtualInstanceBuilder.java
index ccbfb68..5674606 100644
--- a/src/test/java/org/apache/sling/discovery/oak/its/setup/OakVirtualInstanceBuilder.java
+++ b/src/test/java/org/apache/sling/discovery/oak/its/setup/OakVirtualInstanceBuilder.java
@@ -112,7 +112,7 @@ protected ClusterViewService createClusterViewService() {
return OakClusterViewService.testConstructor(getSlingSettingsService(), getResourceResolverFactory(), getIdMapService(), getConfig());
}
- OakTestConfig getConfig() {
+ public OakTestConfig getConfig() {
if (config==null) {
config = createConfig();
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services