imbajin commented on code in PR #2997:
URL: https://github.com/apache/hugegraph/pull/2997#discussion_r3115722488
##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/cassandra/CassandraTest.java:
##########
@@ -192,4 +198,99 @@ public void
testParseReplicaWithNetworkTopologyStrategyAndDoubleReplica() {
Whitebox.invokeStatic(CassandraStore.class, "parseReplica",
config);
});
}
+
+ @Test
+ public void testReconnectOptionsHaveSensibleDefaults() {
+ // Runtime-reconnection options must exist with non-zero defaults so
+ // HugeGraph keeps running when Cassandra restarts (issue #2740).
+ Assert.assertEquals(1000L, (long) CassandraOptions
+ .CASSANDRA_RECONNECT_BASE_DELAY.defaultValue());
+ Assert.assertEquals(60_000L, (long) CassandraOptions
+ .CASSANDRA_RECONNECT_MAX_DELAY.defaultValue());
+ Assert.assertEquals(10, (int) CassandraOptions
+ .CASSANDRA_RECONNECT_MAX_RETRIES.defaultValue());
+ Assert.assertEquals(5000L, (long) CassandraOptions
+ .CASSANDRA_RECONNECT_INTERVAL.defaultValue());
+ }
+
+ @Test
+ public void testReconnectOptionsAreOverridable() {
+ String base = CassandraOptions.CASSANDRA_RECONNECT_BASE_DELAY.name();
+ String max = CassandraOptions.CASSANDRA_RECONNECT_MAX_DELAY.name();
+ String retries = CassandraOptions.CASSANDRA_RECONNECT_MAX_RETRIES
+ .name();
+ String interval = CassandraOptions.CASSANDRA_RECONNECT_INTERVAL.name();
+
+ Configuration conf = new PropertiesConfiguration();
+ conf.setProperty(base, 500L);
+ conf.setProperty(max, 30_000L);
+ conf.setProperty(retries, 3);
+ conf.setProperty(interval, 1000L);
+ HugeConfig config = new HugeConfig(conf);
+
+ Assert.assertEquals(500L, (long) config.get(
+ CassandraOptions.CASSANDRA_RECONNECT_BASE_DELAY));
+ Assert.assertEquals(30_000L, (long) config.get(
+ CassandraOptions.CASSANDRA_RECONNECT_MAX_DELAY));
+ Assert.assertEquals(3, (int) config.get(
+ CassandraOptions.CASSANDRA_RECONNECT_MAX_RETRIES));
+ Assert.assertEquals(1000L, (long) config.get(
+ CassandraOptions.CASSANDRA_RECONNECT_INTERVAL));
+ }
+
+ @Test
+ public void testReconnectRetriesCanBeDisabled() {
+ String retries = CassandraOptions.CASSANDRA_RECONNECT_MAX_RETRIES
+ .name();
+ Configuration conf = new PropertiesConfiguration();
+ conf.setProperty(retries, 0);
+ HugeConfig config = new HugeConfig(conf);
+ Assert.assertEquals(0, (int) config.get(
+ CassandraOptions.CASSANDRA_RECONNECT_MAX_RETRIES));
+ }
+
+ @Test
+ public void testExecuteWithRetrySucceedsAfterTransientFailures() {
+ Configuration conf = new PropertiesConfiguration();
+ HugeConfig config = new HugeConfig(conf);
+ CassandraSessionPool pool = new CassandraSessionPool(config,
Review Comment:
โ ๏ธ **Test constructs `CassandraSessionPool` with a blank config โ fragile
setup**
`new HugeConfig(new PropertiesConfiguration())` produces an empty config.
The constructor calls `config.get(CASSANDRA_RECONNECT_BASE_DELAY)` etc., which
falls back to option defaults only if `HugeConfig` performs that fallback
correctly. If any option's `rangeInt` validator is applied to the absent value
rather than the default, this test will fail with a confusing config error
instead of a clear assertion.
The fields set via `Whitebox.setInternalState` immediately after
construction throw away the constructed values anyway, so the constructor call
is only there to get a valid object โ but it's a fragile path.
Suggested fix: explicitly set the required config values so the test is
self-describing and not dependent on default-fallback behaviour:
```java
Configuration conf = new PropertiesConfiguration();
conf.setProperty(CassandraOptions.CASSANDRA_RECONNECT_BASE_DELAY.name(),
100L);
conf.setProperty(CassandraOptions.CASSANDRA_RECONNECT_MAX_DELAY.name(),
1000L);
conf.setProperty(CassandraOptions.CASSANDRA_RECONNECT_MAX_RETRIES.name(), 3);
conf.setProperty(CassandraOptions.CASSANDRA_RECONNECT_INTERVAL.name(), 1L);
HugeConfig config = new HugeConfig(conf);
```
##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/cassandra/CassandraTest.java:
##########
@@ -192,4 +198,99 @@ public void
testParseReplicaWithNetworkTopologyStrategyAndDoubleReplica() {
Whitebox.invokeStatic(CassandraStore.class, "parseReplica",
config);
});
}
+
+ @Test
+ public void testReconnectOptionsHaveSensibleDefaults() {
+ // Runtime-reconnection options must exist with non-zero defaults so
+ // HugeGraph keeps running when Cassandra restarts (issue #2740).
+ Assert.assertEquals(1000L, (long) CassandraOptions
+ .CASSANDRA_RECONNECT_BASE_DELAY.defaultValue());
+ Assert.assertEquals(60_000L, (long) CassandraOptions
+ .CASSANDRA_RECONNECT_MAX_DELAY.defaultValue());
+ Assert.assertEquals(10, (int) CassandraOptions
+ .CASSANDRA_RECONNECT_MAX_RETRIES.defaultValue());
+ Assert.assertEquals(5000L, (long) CassandraOptions
+ .CASSANDRA_RECONNECT_INTERVAL.defaultValue());
+ }
+
+ @Test
+ public void testReconnectOptionsAreOverridable() {
+ String base = CassandraOptions.CASSANDRA_RECONNECT_BASE_DELAY.name();
+ String max = CassandraOptions.CASSANDRA_RECONNECT_MAX_DELAY.name();
+ String retries = CassandraOptions.CASSANDRA_RECONNECT_MAX_RETRIES
+ .name();
+ String interval = CassandraOptions.CASSANDRA_RECONNECT_INTERVAL.name();
+
+ Configuration conf = new PropertiesConfiguration();
+ conf.setProperty(base, 500L);
+ conf.setProperty(max, 30_000L);
+ conf.setProperty(retries, 3);
+ conf.setProperty(interval, 1000L);
+ HugeConfig config = new HugeConfig(conf);
+
+ Assert.assertEquals(500L, (long) config.get(
+ CassandraOptions.CASSANDRA_RECONNECT_BASE_DELAY));
+ Assert.assertEquals(30_000L, (long) config.get(
+ CassandraOptions.CASSANDRA_RECONNECT_MAX_DELAY));
+ Assert.assertEquals(3, (int) config.get(
+ CassandraOptions.CASSANDRA_RECONNECT_MAX_RETRIES));
+ Assert.assertEquals(1000L, (long) config.get(
+ CassandraOptions.CASSANDRA_RECONNECT_INTERVAL));
+ }
+
+ @Test
+ public void testReconnectRetriesCanBeDisabled() {
+ String retries = CassandraOptions.CASSANDRA_RECONNECT_MAX_RETRIES
+ .name();
+ Configuration conf = new PropertiesConfiguration();
+ conf.setProperty(retries, 0);
+ HugeConfig config = new HugeConfig(conf);
+ Assert.assertEquals(0, (int) config.get(
+ CassandraOptions.CASSANDRA_RECONNECT_MAX_RETRIES));
+ }
+
+ @Test
+ public void testExecuteWithRetrySucceedsAfterTransientFailures() {
+ Configuration conf = new PropertiesConfiguration();
+ HugeConfig config = new HugeConfig(conf);
+ CassandraSessionPool pool = new CassandraSessionPool(config,
+ "ks", "store");
+ Whitebox.setInternalState(pool, "maxRetries", 3);
+ Whitebox.setInternalState(pool, "retryInterval", 1L);
+ Whitebox.setInternalState(pool, "retryMaxDelay", 10L);
+
+ com.datastax.driver.core.Session driverSession = Mockito.mock(
+ com.datastax.driver.core.Session.class);
+ ResultSet rs = Mockito.mock(ResultSet.class);
+ NoHostAvailableException transientFailure =
+ new NoHostAvailableException(Collections.emptyMap());
+ Mockito.when(driverSession.execute(Mockito.any(Statement.class)))
+ .thenThrow(transientFailure)
+ .thenThrow(transientFailure)
+ .thenReturn(rs);
+
+ CassandraSessionPool.Session session = pool.new Session();
+ Whitebox.setInternalState(session, "session", driverSession);
+
+ ResultSet result = session.execute("SELECT now() FROM system.local");
+ Assert.assertSame(rs, result);
+ Mockito.verify(driverSession, Mockito.times(3))
+ .execute(Mockito.any(Statement.class));
+ }
+
+ @Test
+ public void testReconnectOptionsExposeExpectedKeys() {
Review Comment:
๐งน **`testReconnectOptionsExposeExpectedKeys` only checks string literals โ
low-value test**
This test asserts that a static constant's `.name()` returns the exact
string that is defined on the next line of the same class. If someone renames
the key, they update both places together, so this test adds no real safety net.
What is actually worth testing โ and not currently covered โ is that the
validators reject out-of-range values, e.g.:
```java
// base delay below minimum (100 ms) should be rejected
Configuration conf = new PropertiesConfiguration();
conf.setProperty(CassandraOptions.CASSANDRA_RECONNECT_BASE_DELAY.name(),
50L);
Assert.assertThrows(/* config validation exception */, () -> new
HugeConfig(conf));
// max < base should be rejected (the E.checkArgument in the constructor)
conf.setProperty(CassandraOptions.CASSANDRA_RECONNECT_BASE_DELAY.name(),
5000L);
conf.setProperty(CassandraOptions.CASSANDRA_RECONNECT_MAX_DELAY.name(),
1000L);
Assert.assertThrows(IllegalArgumentException.class,
() -> new CassandraSessionPool(new HugeConfig(conf),
"ks", "s"));
```
Consider replacing or augmenting this test with boundary checks like the
above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
