Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NOID] fix uuid flaky routing test #3555

Open
wants to merge 5 commits into
base: 4.4
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 6 additions & 8 deletions full-it/src/test/java/apoc/full/it/CoreExtendedTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@
public class CoreExtendedTest {
@Test
public void checkForCoreAndExtended() {
try {
Neo4jContainerExtension neo4jContainer = createEnterpriseDB(List.of(TestContainerUtil.ApocPackage.FULL), !TestUtil.isRunningInCI())
.withNeo4jConfig("dbms.transaction.timeout", "60s")
try (Neo4jContainerExtension neo4jContainer = createEnterpriseDB(List.of(TestContainerUtil.ApocPackage.FULL), !TestUtil.isRunningInCI())) {

neo4jContainer.withNeo4jConfig("dbms.transaction.timeout", "60s")
.withNeo4jConfig(APOC_IMPORT_FILE_ENABLED, "true");

neo4jContainer.start();
Expand All @@ -62,7 +62,6 @@ public void checkForCoreAndExtended() {
assertTrue(coreCount > 0);
assertTrue(extendedCount > 0);

neo4jContainer.close();
} catch (Exception ex) {
if (TestContainerUtil.isDockerImageAvailable(ex)) {
ex.printStackTrace();
Expand All @@ -73,9 +72,9 @@ public void checkForCoreAndExtended() {

@Test
public void matchesSpreadsheet() {
try {
Neo4jContainerExtension neo4jContainer = createEnterpriseDB(List.of(TestContainerUtil.ApocPackage.FULL), !TestUtil.isRunningInCI())
.withNeo4jConfig("dbms.transaction.timeout", "5s");
try(Neo4jContainerExtension neo4jContainer = createEnterpriseDB(List.of(TestContainerUtil.ApocPackage.FULL), !TestUtil.isRunningInCI())) {

neo4jContainer.withNeo4jConfig("dbms.transaction.timeout", "5s");

neo4jContainer.start();

Expand All @@ -101,7 +100,6 @@ public void matchesSpreadsheet() {

assertEquals(different.toString(), 0, different.size());

neo4jContainer.close();
} catch (Exception ex) {
if (TestContainerUtil.isDockerImageAvailable(ex)) {
ex.printStackTrace();
Expand Down
31 changes: 23 additions & 8 deletions full-it/src/test/java/apoc/full/it/UUIDClusterRoutingTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@
import apoc.util.Neo4jContainerExtension;
import apoc.util.TestContainerUtil;
import apoc.util.TestcontainersCausalCluster;
import org.awaitility.core.ConditionTimeoutException;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Ignore;
import org.junit.Test;
import org.neo4j.driver.*;
import org.neo4j.internal.helpers.collection.Iterators;
Expand Down Expand Up @@ -52,7 +52,7 @@
import static org.neo4j.configuration.GraphDatabaseSettings.SYSTEM_DATABASE_NAME;
import static org.neo4j.test.assertion.Assert.assertEventually;

@Ignore

public class UUIDClusterRoutingTest {
private static final int NUM_CORES = 3;
private static TestcontainersCausalCluster cluster;
Expand Down Expand Up @@ -81,9 +81,10 @@ public static void bringDownCluster() {

@Test
public void testSetupAndDropUuidWithUseSystemClause() {
// wait until members are balanced, i.e. the system LEADER and the neo4j LEADER aren't in the same member
checkLeadershipBalanced(clusterSession);
isLeadershipBalanced();

// no matter if the leader labels are balanced, or not.
// The procedure should always work correctly
queryForEachMembers(members, (session, container) -> {
String label = container.getContainerName();

Expand Down Expand Up @@ -125,14 +126,13 @@ public void testSetupAndDropUuidWithUseSystemClause() {
});

assertEventually(() -> (long) singleResultFirstColumn(cluster.getSession(), countUuids),
(value) -> value == 0L, 10L, TimeUnit.SECONDS);
(value) -> value == 0L, 20L, TimeUnit.SECONDS);

}

@Test
public void testInstallUudiInClusterViaUseSystemShouldFail() {
// wait until members are balanced, i.e. the system LEADER and the neo4j LEADER aren't in the same member
checkLeadershipBalanced(clusterSession);
boolean leadershipBalanced = isLeadershipBalanced();

queryForEachMembers(members, (session, container) -> {
try {
Expand All @@ -143,13 +143,28 @@ public void testInstallUudiInClusterViaUseSystemShouldFail() {
session.writeTransaction(tx -> tx.run(query,
Map.of("label", label) )
);
fail("Should fail because it's not possible to write via deprecated procedure");
// the error happens only if sys db leader is in a different core than the neo4j db leader
if (leadershipBalanced) {
fail("Should fail because it's not possible to write via deprecated procedure");
}
} catch (Exception e) {
assertThat(e.getMessage(), containsString(SYS_NON_LEADER_ERROR));
}
});
}

private boolean isLeadershipBalanced() {
// wait until members are balanced, i.e. the system LEADER and the neo4j LEADER aren't in the same member.
// Some time, although causal_clustering.leadership_balancing=NO_BALANCING,
// the system db leader is placed in the same core as the neo4j db leader
try {
checkLeadershipBalanced(clusterSession);
return true;
} catch (ConditionTimeoutException e) {
return false;
}
}

@Test
public void testUuidInstallAllowedOnlyInSysLeaderMember() {
final String query = "CALL apoc.uuid.install($label)";
Expand Down
5 changes: 3 additions & 2 deletions test-utils/src/main/java/apoc/util/TestContainerUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import org.apache.commons.io.filefilter.TrueFileFilter;
import org.apache.commons.io.filefilter.WildcardFileFilter;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.awaitility.core.ConditionTimeoutException;
import org.gradle.tooling.BuildLauncher;
import org.gradle.tooling.GradleConnector;
import org.gradle.tooling.ProjectConnection;
Expand Down Expand Up @@ -286,7 +287,7 @@ public static <T> T singleResultFirstColumn(Session session, String cypher) {
*
* @param session
*/
public static void checkLeadershipBalanced(Session session) {
public static void checkLeadershipBalanced(Session session) throws ConditionTimeoutException {
assertEventually(() -> {
String query = "CALL dbms.cluster.overview() YIELD databases\n" +
"WITH databases.neo4j AS neo4j, databases.system AS system\n" +
Expand All @@ -300,7 +301,7 @@ public static void checkLeadershipBalanced(Session session) {
return false;
}
},
(value) -> value, 60L, TimeUnit.SECONDS);
(value) -> value, 30L, TimeUnit.SECONDS);
}

/**
Expand Down