dcapwell commented on code in PR #2133:
URL: https://github.com/apache/cassandra/pull/2133#discussion_r1146567885
##########
src/java/org/apache/cassandra/config/DatabaseDescriptor.java:
##########
@@ -1504,137 +1599,131 @@ public static void setRoleManager(IRoleManager
roleManager)
public static int getPermissionsValidity()
{
- return conf.permissions_validity.toMilliseconds();
+ return getProperty(DurationSpec.IntMillisecondsBound.class,
ConfigFields.PERMISSIONS_VALIDITY).toMilliseconds();
Review Comment:
please revert, `getProperty` is very expensive, and this is all for internal
access...
##########
src/java/org/apache/cassandra/config/DatabaseDescriptor.java:
##########
@@ -1504,137 +1599,131 @@ public static void setRoleManager(IRoleManager
roleManager)
public static int getPermissionsValidity()
{
- return conf.permissions_validity.toMilliseconds();
+ return getProperty(DurationSpec.IntMillisecondsBound.class,
ConfigFields.PERMISSIONS_VALIDITY).toMilliseconds();
}
public static void setPermissionsValidity(int timeout)
{
- conf.permissions_validity = new
DurationSpec.IntMillisecondsBound(timeout);
+ setProperty(ConfigFields.PERMISSIONS_VALIDITY, new
DurationSpec.IntMillisecondsBound(timeout));
}
public static int getPermissionsUpdateInterval()
{
- return conf.permissions_update_interval == null
- ? conf.permissions_validity.toMilliseconds()
- : conf.permissions_update_interval.toMilliseconds();
+ return getProperty(DurationSpec.IntMillisecondsBound.class,
ConfigFields.PERMISSIONS_UPDATE_INTERVAL) == null
+ ? getPermissionsValidity() :
+ getProperty(DurationSpec.IntMillisecondsBound.class,
ConfigFields.PERMISSIONS_UPDATE_INTERVAL).toMilliseconds();
}
public static void setPermissionsUpdateInterval(int updateInterval)
{
- if (updateInterval == -1)
- conf.permissions_update_interval = null;
- else
- conf.permissions_update_interval = new
DurationSpec.IntMillisecondsBound(updateInterval);
+ DurationSpec.IntMillisecondsBound interval = updateInterval == -1 ?
null : new DurationSpec.IntMillisecondsBound(updateInterval);
+ setProperty(ConfigFields.PERMISSIONS_UPDATE_INTERVAL, interval);
}
public static int getPermissionsCacheMaxEntries()
{
- return conf.permissions_cache_max_entries;
+ return getProperty(Integer.TYPE,
ConfigFields.PERMISSIONS_CACHE_MAX_ENTRIES);
}
- public static int setPermissionsCacheMaxEntries(int maxEntries)
+ public static void setPermissionsCacheMaxEntries(int maxEntries)
{
- return conf.permissions_cache_max_entries = maxEntries;
+ setProperty(ConfigFields.PERMISSIONS_CACHE_MAX_ENTRIES, maxEntries);
}
public static boolean getPermissionsCacheActiveUpdate()
{
- return conf.permissions_cache_active_update;
+ return getProperty(Boolean.TYPE,
ConfigFields.PERMISSIONS_CACHE_ACTIVE_UPDATE);
}
public static void setPermissionsCacheActiveUpdate(boolean update)
{
- conf.permissions_cache_active_update = update;
+ setProperty(ConfigFields.PERMISSIONS_CACHE_ACTIVE_UPDATE, update);
}
public static int getRolesValidity()
{
- return conf.roles_validity.toMilliseconds();
+ return getProperty(DurationSpec.IntMillisecondsBound.class,
ConfigFields.ROLES_VALIDITY).toMilliseconds();
}
public static void setRolesValidity(int validity)
{
- conf.roles_validity = new DurationSpec.IntMillisecondsBound(validity);
+ setProperty(ConfigFields.ROLES_VALIDITY, new
DurationSpec.IntMillisecondsBound(validity));
}
public static int getRolesUpdateInterval()
{
- return conf.roles_update_interval == null
- ? conf.roles_validity.toMilliseconds()
- : conf.roles_update_interval.toMilliseconds();
+ return getProperty(DurationSpec.IntMillisecondsBound.class,
ConfigFields.ROLES_UPDATE_INTERVAL) == null
+ ? getRolesValidity() :
+ getProperty(DurationSpec.IntMillisecondsBound.class,
ConfigFields.ROLES_UPDATE_INTERVAL).toMilliseconds();
}
public static void setRolesCacheActiveUpdate(boolean update)
{
- conf.roles_cache_active_update = update;
+ setProperty(ConfigFields.ROLES_CACHE_ACTIVE_UPDATE, update);
}
public static boolean getRolesCacheActiveUpdate()
{
- return conf.roles_cache_active_update;
+ return getProperty(Boolean.TYPE,
ConfigFields.ROLES_CACHE_ACTIVE_UPDATE);
}
public static void setRolesUpdateInterval(int interval)
{
- if (interval == -1)
- conf.roles_update_interval = null;
- else
- conf.roles_update_interval = new
DurationSpec.IntMillisecondsBound(interval);
+ DurationSpec.IntMillisecondsBound updateInterval = interval == -1 ?
null : new DurationSpec.IntMillisecondsBound(interval);
+ setProperty(ConfigFields.ROLES_UPDATE_INTERVAL, updateInterval);
}
public static int getRolesCacheMaxEntries()
{
- return conf.roles_cache_max_entries;
+ return getProperty(Integer.TYPE, ConfigFields.ROLES_CACHE_MAX_ENTRIES);
}
- public static int setRolesCacheMaxEntries(int maxEntries)
+ public static void setRolesCacheMaxEntries(int maxEntries)
Review Comment:
please revert, unrelated code changes goes against our style guide
##########
src/java/org/apache/cassandra/config/DatabaseDescriptor.java:
##########
@@ -4676,4 +4729,41 @@ public static void
setClientRequestSizeMetricsEnabled(boolean enabled)
{
return Objects.requireNonNull(sstableFormatFactories, "Forgot to
initialize DatabaseDescriptor?");
}
+
+ /**
+ * Set configuration property for the given name to {@link #confRegistry}
if a safe manner
+ * with handling internal Cassandra exceptions.
+ *
+ * @param name Property name.
+ * @param value Property value.
+ */
+ private static void setProperty(String name, Object value)
+ {
+ runExceptionally(() -> confRegistry.set(name, value), new
SearchInternalCauseForPublicAPI());
+ }
+
+ private static <T> T getProperty(Class<T> cls, String name)
+ {
+ return callExceptionally(() -> confRegistry.get(cls, name), new
SearchInternalCauseForPublicAPI());
+ }
+
+ private static class SearchInternalCauseForPublicAPI implements
Function<Exception, RuntimeException>
+ {
+ @Override
+ public RuntimeException apply(Exception e)
+ {
+ RuntimeException rt;
+ if ((rt = cause(e, IllegalArgumentException.class)) != null)
+ return new IllegalArgumentException(rt.getMessage());
+ else if ((rt = cause(e, IllegalStateException.class)) != null)
+ return new IllegalStateException(rt.getMessage());
+ else if ((rt = cause(e, UnsupportedOperationException.class)) !=
null)
+ return new UnsupportedOperationException(rt.getMessage());
+ else
+ {
+ logger.error("Unexpected exception", e);
+ return new RuntimeException(e.getMessage());
+ }
+ }
+ }
Review Comment:
Why does this exist?
##########
src/java/org/apache/cassandra/config/DatabaseDescriptor.java:
##########
@@ -3105,12 +3193,14 @@ public static Set<String> hintedHandoffDisabledDCs()
public static boolean useDeterministicTableID()
{
- return conf != null && conf.use_deterministic_table_id;
+ if (confRegistry == null)
Review Comment:
please revert
--
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]
