frankgh commented on code in PR #3706:
URL: https://github.com/apache/cassandra/pull/3706#discussion_r1883965722
##########
src/java/org/apache/cassandra/auth/CassandraRoleManager.java:
##########
@@ -732,4 +737,32 @@ public Supplier<Map<RoleResource, Set<Role>>> bulkLoader()
return entries;
};
}
+
+ protected void scheduleDisconnectInvalidRoleTask()
+ {
+ // Delay between executions, with jitter
+ int period = (int)
DatabaseDescriptor.getInvalidClientDisconnectPeriod(TimeUnit.MILLISECONDS);
+ int jitter = (int)
DatabaseDescriptor.getInvalidClientDisconnectJitter(TimeUnit.MILLISECONDS);
Review Comment:
use long for period + jitter
##########
src/java/org/apache/cassandra/config/DatabaseDescriptor.java:
##########
@@ -5349,4 +5349,26 @@ public static boolean
isPasswordValidatorReconfigurationEnabled()
{
return conf.password_validator_reconfiguration_enabled;
}
+
+ public static long getInvalidClientDisconnectPeriod(TimeUnit unit)
+ {
+ return conf.invalid_client_disconnect_period.to(unit);
+ }
+
+ public static void setInvalidClientDisconnectPeriod(TimeUnit unit, long
duration)
+ {
+ logger.info("invalid_client_disconnect_period set to {} {}", duration,
unit);
+ conf.invalid_client_disconnect_period = new
DurationSpec.LongMillisecondsBound(duration, unit);
+ }
+
+ public static long getInvalidClientDisconnectJitter(TimeUnit unit)
+ {
+ return conf.invalid_client_disconnect_jitter.to(unit);
+ }
+
+ public static void setInvalidClientDisconnectJitter(TimeUnit unit, long
duration)
+ {
+ logger.info("invalid_client_disconnect_jitter set to {} {}", duration,
unit);
Review Comment:
same here.
##########
src/java/org/apache/cassandra/auth/CassandraRoleManager.java:
##########
@@ -732,4 +737,32 @@ public Supplier<Map<RoleResource, Set<Role>>> bulkLoader()
return entries;
};
}
+
+ protected void scheduleDisconnectInvalidRoleTask()
+ {
+ // Delay between executions, with jitter
+ int period = (int)
DatabaseDescriptor.getInvalidClientDisconnectPeriod(TimeUnit.MILLISECONDS);
+ int jitter = (int)
DatabaseDescriptor.getInvalidClientDisconnectJitter(TimeUnit.MILLISECONDS);
+ if (period < 0)
+ {
+ logger.info("Skipping invalid role disconnection");
+ return;
+ }
+ IntSupplier delayMillis = () -> period +
ThreadLocalRandom.current().nextInt(0, jitter);
Review Comment:
This should be a LongSupplier. In general, delay should be supplied as
long, for consistency we should stick to long here as well.
##########
src/java/org/apache/cassandra/config/DatabaseDescriptor.java:
##########
@@ -5349,4 +5349,26 @@ public static boolean
isPasswordValidatorReconfigurationEnabled()
{
return conf.password_validator_reconfiguration_enabled;
}
+
+ public static long getInvalidClientDisconnectPeriod(TimeUnit unit)
+ {
+ return conf.invalid_client_disconnect_period.to(unit);
+ }
+
+ public static void setInvalidClientDisconnectPeriod(TimeUnit unit, long
duration)
Review Comment:
are we expecting to update these values from somewhere? I don't see any call
sites for this method
##########
src/java/org/apache/cassandra/config/DatabaseDescriptor.java:
##########
@@ -5349,4 +5349,26 @@ public static boolean
isPasswordValidatorReconfigurationEnabled()
{
return conf.password_validator_reconfiguration_enabled;
}
+
+ public static long getInvalidClientDisconnectPeriod(TimeUnit unit)
+ {
+ return conf.invalid_client_disconnect_period.to(unit);
+ }
+
+ public static void setInvalidClientDisconnectPeriod(TimeUnit unit, long
duration)
+ {
+ logger.info("invalid_client_disconnect_period set to {} {}", duration,
unit);
Review Comment:
I find it useful to know what was the previous value as well during
debugging, so if we could include that, it'd be great. i.e.
`invalid_client_disconnect_period set from <previous> to <new>`
##########
src/java/org/apache/cassandra/auth/CassandraRoleManager.java:
##########
@@ -732,4 +737,32 @@ public Supplier<Map<RoleResource, Set<Role>>> bulkLoader()
return entries;
};
}
+
+ protected void scheduleDisconnectInvalidRoleTask()
+ {
+ // Delay between executions, with jitter
+ int period = (int)
DatabaseDescriptor.getInvalidClientDisconnectPeriod(TimeUnit.MILLISECONDS);
Review Comment:
I see methods to set these properties (period+jitter), however if you
statically retrieve the value once, setting them anywhere else will have no
effect. So if the desire is to be able to have some mechanism to update these
values while the process is running (i.e. JMX), we should probably retrieve
these values dynamically as well.
--
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]
