[
https://issues.apache.org/jira/browse/CASSANDRA-15254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17685424#comment-17685424
]
David Capwell commented on CASSANDRA-15254:
-------------------------------------------
bq. Use the DatabaseDescriptor/GuardrailsOptions setter/getter methods directly
in auto-generated classes to set a relationship between the property name and
its setter/getter methods;
bq. Pros: The right methods are used to access Config, easy to debug in case of
errors, easy to see setter/getter method usages and classes dependencies, type
safety since the type of property ;
bq. Cons: Require additional utils to generate such classes, the additional
classes themselves increase the size of PR;
I feel this is very subjective and assumes that there is a well known "right"
method; there isn't. It's not uncommon that a single property has multiple
getters or setters in DatabaseDescriptor (DD), so how do you choose the "right"
method? What does "right" even mean?
I also feel this doesn't really propose that "auto-generated" is a con, but I
feel that it can be. Authors must know how this logic works, be able to debug
and inspect it, and make sure it works in their IDEs; this adds a ton of
complexity.
bq. Set/get a Config's field through the SnakeYaml Property class by using
reflection under the hood;
bq. Pros: Easy to implement with minimal changes;
bq. Cons: harder to understand how the config is being accessed and
manipulated, may violate the contract of config management passing by
getter/setter methods, not easy to debug and cause in case of failure,
reflective access to fields can bypass any validations or restrictions;
I don't actually agree with these cons. Right now we only mutate via JMX and
have logging to say this happened, why could the settings table not offer the
same? As a developer or operator, do you actually care what method was called
or do you care that the config X was set to 42?
bq. may violate the contract of config management passing by getter/setter
methods
I don't follow what this means; what "contract"?
bq. not easy to debug and cause in case of failure
How is that any different than the 2 other options above? How is setting a
field harder to debug than setting a method that some tooling choose as the
"right" method? I personally find it less confusing as there are no
abstractions in the way; you just set a field (though have to use reflection).
bq. reflective access to fields can bypass any validations or restrictions;
This has been something [~e.dimitrova] has been working to improve in many
configs as how "validation" works isn't consistent and several JMX calls would
bypass all validation (or have different logic than YAML)! The leading idea
that I know right now is to push this into the type system; don't allow the
config to be generated. You see this behavior in stuff like
DurationSpec.IntMillisecondsBound where you are not capable of creating the
type if it's not valid.
bq. I chose option 1 because it provides much more code clarity for users and
developers, which I think is more important than its drawbacks.
One thing that must be thought about is the experience of adding or changing a
config, this is actually very critical for adoption and making sure the system
behaves correctly.
Right now the pattern is to update Config and add at least a getter in DD for
static access (DD holds the Config reference). At this point I am done and am
able to start using my config within my code.
Now, lets say I need this config to be "dynamic", because JMX is the current
solution you create a setter in DD, make the field volatile, then find the
"correct" (can be subjective) MBean to put the getter/setter into... this is
tedious...
Now, let's say you need to change the name or type (or both) of a config, what
do you do? You rename Config's field and add a "@Replaces" annotation, then
the config system makes sure everything is handled properly...
Now we get back to this patch, the focus is to replace (and live next to) the
JMX way, so what do I as an author need to do to integrate with this system?
The current JMX solution is already seen as too much work, so adding more is
something I think is a big issue as it makes people opt-in which is hard for
adoption. So when we talk about solutions, we must also weigh the effort on
authors to integrate.
This makes me concerned about #1 and #2 as they feel like more work to me. For
#1 I need to make sure that the inference system does the right thing, but the
only user facing rules are the names in Config, so if I have a typo in DD does
this leak out? Do I as a reviewer/author need to try to look super close as
anything we do must be maintained? What about "@Replaced" configs? They don't
have a DD getter/setters present, so now we need this system to understand and
find the "correct" DD methods and deal with conversions? I have so much more
questions on the process, and since the patch doesn't actually have anything to
"auto-generate" code, this all looks manual, so far more work than JMX today.
bq. The plan is only to allow setting the properties that can be set
dynamically today (which is a small sub-set of the properties), right?
This sadly is an annoying problem that [~e.dimitrova] pointed out at the start
of this ticket... At the config layer we don't have feedback on if something is
mutable or not, its 100% dependent on the caller... We have cases that the
caller "saves" the output into a static, causing any config change to go
ignored... I think she mentioned trying to avoid these caches as a solution
but don't remember if there was any attempt to push forward with that...
This is something that should be thought about as it just leads to confusion
for operators asking why behaviors were not changed when the config system
shows the update.
bq. I'd also like to clarify my term "auto-generated classes" for better
understanding. Such classes based on DatabaseDescriptor/GuardrailsOptions
metadata will not be generated every time on build or at runtime. I plan to
provide a util class that will create a new class body that implements all the
necessary interfaces (SetterWalker, GetterWalker) that we rely on and will be
committed as a regular class to remove the manual work of coupling a property
name with its getter/setter methods.
I feel that this is a bad idea. If you want to "auto-generate" something it
"should" happen during every build. If you ask users to opt-in then it won't
happen... Simple example is my JMX validation tests... outside of me who runs
to produce new snapshots? Do we have snapshots for 4.1? By not doing this
step in the build you ask authors to "do the right thing" and I feel this will
be forgotten majority of the time.
> 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
