[
https://issues.apache.org/jira/browse/CASSANDRA-9425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15827747#comment-15827747
]
Sylvain Lebresne commented on CASSANDRA-9425:
---------------------------------------------
bq. and undid a tiny bit of of the related CASSANDRA-10410 optimisation I hope
you don’t mind.
Well, actually, your motivations on this are lost on me. I'm certainly not
pretending that optimization was the most important thing ever, but as
mentioned in the original ticket it did show up in some profiling and the
"optimization" was beyond tiny and entirely encapsulated within the class. As
an aside, also not a fan of manually writting hashcode() methods when the
simpler Objects.hashcode() can be and was used.
bq. Problem is, in announce for CREATE INDEX we do not call KSM.validate(),
yet. So for the time being, until the next patch, where we always validate the
entirety of the resulting schema before applying anything, it has to stay here,
even if imperfect.
Wfm, though it sounds like something we could add as comment in the code, so we
remember what's up in the unlikely even that "next patch" forgets about this
(and so I don't have to ask about this removal when reviewing the next patch
since I'll most likely will have forgot about it).
bq. we should probably eventually move their processing elsewhere, away from
the sensitive path
I don't necessarily disagree but I don't think it's a big problem right now so
I'd suggest keeping it simple for now.
bq. It’s a very minor case of duplication that doesn’t worry me, I’d rather not
factor it wouldn’t strictly speaking make things simpler.
I think that, all other things being equal, terser and easier to read code
*does* make things simpler, even if it's a tiny bit so, so I still stand by my
suggestion. But it's certainly small and probably up to personal preferences,
so ok.
bq. It’s an ugly pre-existing piece of code to avoid re-compiling the UDFs; It
annoys me immensely that a method in SchemaKeyspace is referencing
Schema.instance, hence the TODO. I want to get rid of it eventually, but it’s
not important enough to spend my time on it atm.
Fair enough, but could you update the TODO with this expanded and useful
context.
bq. The alternative would be using {{get(name).orElse(null)}}
Well, that's one alternative. The other, which I'd personally would go with, is
to remove the method that return {{Optional}} altogether. {{Optional}} is
useless outside of adding clarity to the fact there may be no result, but
naming the method {{getNullable}} is good enough on that front imo.
Thanks for the explanations on the rest.
> Make node-local schema fully immutable
> --------------------------------------
>
> Key: CASSANDRA-9425
> URL: https://issues.apache.org/jira/browse/CASSANDRA-9425
> Project: Cassandra
> Issue Type: Sub-task
> Reporter: Aleksey Yeschenko
> Assignee: Aleksey Yeschenko
> Fix For: 4.0
>
>
> The way we handle schema changes currently is inherently racy.
> All of our {{SchemaAlteringStatement}} s perform validation on a schema state
> that's won't necessarily be there when the statement gets executed and
> mutates schema.
> We should make all the *Metadata classes ({{KeyspaceMetadata,
> TableMetadata}}, {{ColumnMetadata}}, immutable, and local schema persistently
> snapshottable, with a single top-level {{AtomicReference}} to the current
> snapshot. Have DDL statements perform validation and transformation on the
> same state.
> In pseudo-code, think
> {code}
> public interface DDLStatement
> {
> /**
> * Validates that the DDL statement can be applied to the provided schema
> snapshot.
> *
> * @param schema snapshot of schema before executing CREATE KEYSPACE
> */
> void validate(SchemaSnapshot schema);
>
> /**
> * Applies the DDL statement to the provided schema snapshot.
> * Implies that validate() has already been called on the provided
> snapshot.
> *
> * @param schema snapshot of schema before executing the statement
> * @return snapshot of schema as it would be after executing the statement
> */
> SchemaSnapshot transform(SchemaSnapshot schema);
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
