[
https://issues.apache.org/jira/browse/CASSANDRA-15254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17686080#comment-17686080
]
David Capwell commented on CASSANDRA-15254:
-------------------------------------------
bq. Once such a property is loaded, its old name is no longer available at
runtime for usage, right?
Incorrect, they are exposed in the settings table. The JMX names (for
backwards compatibility reasons) tend to be the old names and not the new
names... so on a rename you tend to be lacking JMX names that match the new
name, and the settings table exposes both. DD may use w/e it wants as the
names don't matter to authors.
bq. it is correct not to have setter and getter methods for them in the
DatabaseDescriptor class.
To lower the size of a diff, its not uncommon for DD to be left alone on
renames (outside of changing the field referenced), so DD may have the old
name, the new name, or any other name. So it is not "correct" to say that it's
not "correct" for DD to reference older names.
bq. The properties that are NOT changed at runtime.
bq. An example of such a property might be Config.cluster_name or
Config.storage_port. These types of properties are not changed during a node
lifecycle and MUST NOT be changed at runtime.
Yep, those are some examples.
bq. The properties that can be changed at runtime and are responsible for
managing a node's internal processes or state during the node lifecycle.
what does this mean?
bq. No matter how we intend to update a particular property using the setter
method or reflection, it's currently impossible to determine from the source
code we have whether that property is updatable or not
It's complex to get a 100% correct answer (need something to look at all code
paths), but just checking if a field is volatile solves for 80% of the cases.
You might mutate something that isn't mutable (we can always just remove
volatile in those known cases), and some people make a field mutable without
volatile... but that isn't 100% safe...
So yeah, the cheapest way is to check for "volatile"; its not 100% correct but
generally works
bq. Enforce the contract between the filed name and the setter method name,
with the field type and the method input type. It differs now for almost
everything - setRepairRpcTimeout and repair_request_timeout, but we must have
setRepairRequestTimeout and repair_request_timeout to ensure the field is
changeable;
does this mean the build fails if we mess up and can't have a match? Any idea
how many methods don't match the "new" name?
To see the impact, wrote a quick test to show the current state.
{code}
@Test
public void test()
{
Field[] fields = Config.class.getDeclaredFields();
Method[] methods = DatabaseDescriptor.class.getDeclaredMethods();
long empty = 0, immutable = 0, mutable = 0;
for (Field field : fields)
{
if (Modifier.isStatic(field.getModifiers())) continue;
String name = FBUtilities.snakeToCamel(field.getName());
name = name.substring(0, 1).toUpperCase() + name.substring(1);
String getter = "get" + name;
String setter = "set" + name;
long numGetters = Stream.of(methods).filter(m ->
m.getName().equals(getter)).count();
long numSetters = Stream.of(methods).filter(m ->
m.getName().equals(setter)).count();
if (numGetters == 0)
{
System.out.println("No getter found for " + name);
empty++;
}
else if (numSetters == 0)
{
System.out.println("Immutable " + name);
immutable++;
}
else
{
System.out.println("Mutable " + name);
mutable++;
}
}
double total = empty + mutable + immutable;
System.out.println("Empty configs: " + empty + "(" + (empty / total *
100) + "%)");
System.out.println("Immutable configs: " + empty + "(" + (immutable /
total * 100) + "%)");
System.out.println("Mutable configs: " + empty + "(" + (mutable / total
* 100) + "%)");
}
{code}
Here we see the following sample cases that stand out
{code}
INFO [main] 2023-02-08 11:21:10,176 SubstituteLogger.java:169 - Immutable
ClusterName
INFO [main] 2023-02-08 11:21:10,177 SubstituteLogger.java:169 - Mutable
Authenticator // not actually mutable, used only in
org.apache.cassandra.auth.AuthConfig#applyAuth
INFO [main] 2023-02-08 11:21:10,178 SubstituteLogger.java:169 - No getter
found for AutoBootstrap
INFO [main] 2023-02-08 11:21:10,178 SubstituteLogger.java:169 - No getter
found for HintedHandoffEnabled
INFO [main] 2023-02-08 11:21:10,178 SubstituteLogger.java:169 - No getter
found for HintedHandoffDisabledDatacenters
INFO [main] 2023-02-08 11:21:10,179 SubstituteLogger.java:169 - No getter
found for RequestTimeout
...
INFO [main] 2023-02-08 11:29:42,744 SubstituteLogger.java:169 - Empty configs:
222(61.495844875346265%)
INFO [main] 2023-02-08 11:29:42,744 SubstituteLogger.java:169 - Immutable
configs: 222(9.418282548476455%)
INFO [main] 2023-02-08 11:29:42,744 SubstituteLogger.java:169 - Mutable
configs: 222(29.085872576177284%)
{code}
With this I see that ~30% of configs match this comments definition of mutable,
but showed cases where that was not actually true.
Around 61% of cases don't have getters or setters! This will boil down to the
earlier comment I was making, that there isn't a 1:1 mapping between config
name and DD...
Now, "could" we be smarter about the matching? sure... but was trying to point
out this is non-trivial at the moment.
bq. Getting and setting fields in the Config class is done by
org.yaml.snakeyaml.introspector.Property in the ConfigurationRegistry, as you
suggested in the comments earlier;
slight correction... Config supports non-fields and allows exposing
getter/setters instead... no one uses this in Config but it's used in some of
the nested types. Property abstracts this and uses Field if that's what we
need, or falls back to getter/setter if those are present.
bq. We will be able to remove annoying MBean classes
Won't be able to do that, can't break compatibility.
> Allow UPDATE on settings virtual table to change running configurations
> -----------------------------------------------------------------------
>
> Key: CASSANDRA-15254
> URL: https://issues.apache.org/jira/browse/CASSANDRA-15254
> Project: Cassandra
> Issue Type: New Feature
> Components: Feature/Virtual Tables
> Reporter: Chris Lohfink
> Assignee: Maxim Muzafarov
> Priority: Normal
> Attachments: Configuration Registry Diagram.png
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> Allow using UPDATE on the system_views.settings virtual table to update
> configs at runtime for the equivalent of the dispersed JMX
> attributes/operations.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
