You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "jacek-lewandowski (via GitHub)" <gi...@apache.org> on 2023/10/11 09:46:49 UTC
[PR] CASSANDRA-18921-4.1: Fix DescribeStatement [cassandra]
jacek-lewandowski opened a new pull request, #2793:
URL: https://github.com/apache/cassandra/pull/2793
(no comment)
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18921-4.1: Fix DescribeStatement [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2793:
URL: https://github.com/apache/cassandra/pull/2793#discussion_r1356655262
##########
src/java/org/apache/cassandra/cql3/statements/DescribeStatement.java:
##########
@@ -132,8 +132,13 @@ public final ResultMessage execute(QueryState state, QueryOptions options, long
@Override
public ResultMessage executeLocally(QueryState state, QueryOptions options)
{
- Keyspaces keyspaces = Schema.instance.distributedAndLocalKeyspaces();
- UUID schemaVersion = Schema.instance.getVersion();
+ UUID schemaVersion;
+ Keyspaces keyspaces;
+ do
+ {
+ schemaVersion = Schema.instance.getVersionSync();
+ keyspaces = Schema.instance.distributedAndLocalKeyspaces();
Review Comment:
Ah never mind ... I was reading this change wrongly. `keyspaces` are used after this do-while ... But anyway, does `keyspaces = ... ` have to be in that do-while too?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18921-4.1: Fix DescribeStatement [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2793:
URL: https://github.com/apache/cassandra/pull/2793#discussion_r1356653028
##########
src/java/org/apache/cassandra/cql3/statements/DescribeStatement.java:
##########
@@ -132,8 +132,13 @@ public final ResultMessage execute(QueryState state, QueryOptions options, long
@Override
public ResultMessage executeLocally(QueryState state, QueryOptions options)
{
- Keyspaces keyspaces = Schema.instance.distributedAndLocalKeyspaces();
- UUID schemaVersion = Schema.instance.getVersion();
+ UUID schemaVersion;
+ Keyspaces keyspaces;
+ do
+ {
+ schemaVersion = Schema.instance.getVersionSync();
+ keyspaces = Schema.instance.distributedAndLocalKeyspaces();
Review Comment:
@jacek-lewandowski this seems interesting at first sight ... feels unnatural. Basically, we set `keyspaces` but they are not used in `while`. Is that `distributedAndLocalKeyspaces` doing something which will make `getVersionSync` behave differently? Just asking ...
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18921-4.1: Fix DescribeStatement [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2793:
URL: https://github.com/apache/cassandra/pull/2793#discussion_r1356653028
##########
src/java/org/apache/cassandra/cql3/statements/DescribeStatement.java:
##########
@@ -132,8 +132,13 @@ public final ResultMessage execute(QueryState state, QueryOptions options, long
@Override
public ResultMessage executeLocally(QueryState state, QueryOptions options)
{
- Keyspaces keyspaces = Schema.instance.distributedAndLocalKeyspaces();
- UUID schemaVersion = Schema.instance.getVersion();
+ UUID schemaVersion;
+ Keyspaces keyspaces;
+ do
+ {
+ schemaVersion = Schema.instance.getVersionSync();
+ keyspaces = Schema.instance.distributedAndLocalKeyspaces();
Review Comment:
@jacek-lewandowski this seems interesting at first sight ... feels unnatural. Basically, we set `keyspaces` but they are not used in `while`.
does `keyspaces = ... ` have to be in that do-while too? Do I understand it right that calling `distributedAndLocalKeyspaces` will change `getVersionSync` eventually too?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18921-4.1: Fix DescribeStatement [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2793:
URL: https://github.com/apache/cassandra/pull/2793#discussion_r1356655850
##########
src/java/org/apache/cassandra/schema/Schema.java:
##########
@@ -496,25 +496,43 @@ public Optional<Function> findFunction(FunctionName name, List<AbstractType<?>>
/* Version control */
/**
- * @return current schema version
+ * Returns the current schema version. Although, if the schema is being updated while the method was called, it
+ * can return a stale version which does not correspond to the current keyspaces metadata. It is because the schema
+ * version is unknown for the partially applied changes and is updated after the entire schema change is applied.
+ * @see #getVersionSync()
*/
public UUID getVersion()
{
return version;
}
+ /**
+ * Returns the current schema version. If the schema is in the middle of the update, the method will wait until
+ * the update is completed and return the new version.
+ */
+ public synchronized UUID getVersionSync()
+ {
+ return version;
+ }
+
/**
* Checks whether the given schema version is the same as the current local schema.
+ * <p>
+ * @deprecated please use {@link #getVersionSync()} or {@link #getVersion()} depending on the use case
*/
- public boolean isSameVersion(UUID schemaVersion)
+ @Deprecated(since = "4.1", forRemoval = true)
Review Comment:
:1st_place_medal:
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18921-4.1: Fix DescribeStatement [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2793:
URL: https://github.com/apache/cassandra/pull/2793#discussion_r1356653028
##########
src/java/org/apache/cassandra/cql3/statements/DescribeStatement.java:
##########
@@ -132,8 +132,13 @@ public final ResultMessage execute(QueryState state, QueryOptions options, long
@Override
public ResultMessage executeLocally(QueryState state, QueryOptions options)
{
- Keyspaces keyspaces = Schema.instance.distributedAndLocalKeyspaces();
- UUID schemaVersion = Schema.instance.getVersion();
+ UUID schemaVersion;
+ Keyspaces keyspaces;
+ do
+ {
+ schemaVersion = Schema.instance.getVersionSync();
+ keyspaces = Schema.instance.distributedAndLocalKeyspaces();
Review Comment:
@jacek-lewandowski this seems interesting at first sight ... feels unnatural. Basically, we set `keyspaces` but they are not used in `while`.
does `keyspaces = ... ` have to be in that do-while too?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18921-4.1: Fix DescribeStatement [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic closed pull request #2793: CASSANDRA-18921-4.1: Fix DescribeStatement
URL: https://github.com/apache/cassandra/pull/2793
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18921-4.1: Fix DescribeStatement [cassandra]
Posted by "jacek-lewandowski (via GitHub)" <gi...@apache.org>.
jacek-lewandowski commented on PR #2793:
URL: https://github.com/apache/cassandra/pull/2793#issuecomment-1782712328
https://app.circleci.com/pipelines/github/jacek-lewandowski/cassandra/1040/workflows/a44e95b0-67a5-4e89-88f6-2680dedd80e0 🆗
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18921-4.1: Fix DescribeStatement [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2793:
URL: https://github.com/apache/cassandra/pull/2793#discussion_r1356655262
##########
src/java/org/apache/cassandra/cql3/statements/DescribeStatement.java:
##########
@@ -132,8 +132,13 @@ public final ResultMessage execute(QueryState state, QueryOptions options, long
@Override
public ResultMessage executeLocally(QueryState state, QueryOptions options)
{
- Keyspaces keyspaces = Schema.instance.distributedAndLocalKeyspaces();
- UUID schemaVersion = Schema.instance.getVersion();
+ UUID schemaVersion;
+ Keyspaces keyspaces;
+ do
+ {
+ schemaVersion = Schema.instance.getVersionSync();
+ keyspaces = Schema.instance.distributedAndLocalKeyspaces();
Review Comment:
Ah never mind ... I was reading this change wrongly. `keyspaces` are used after this do-while ... But anyway, does `keyspaces = ... ` have to be in that do-while too?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18921-4.1: Fix DescribeStatement [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2793:
URL: https://github.com/apache/cassandra/pull/2793#discussion_r1356655262
##########
src/java/org/apache/cassandra/cql3/statements/DescribeStatement.java:
##########
@@ -132,8 +132,13 @@ public final ResultMessage execute(QueryState state, QueryOptions options, long
@Override
public ResultMessage executeLocally(QueryState state, QueryOptions options)
{
- Keyspaces keyspaces = Schema.instance.distributedAndLocalKeyspaces();
- UUID schemaVersion = Schema.instance.getVersion();
+ UUID schemaVersion;
+ Keyspaces keyspaces;
+ do
+ {
+ schemaVersion = Schema.instance.getVersionSync();
+ keyspaces = Schema.instance.distributedAndLocalKeyspaces();
Review Comment:
Ah never mind ... I was reading this change wrongly. `keyspaces` are used after this do-while ... Forget it.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18921-4.1: Fix DescribeStatement [cassandra]
Posted by "jacek-lewandowski (via GitHub)" <gi...@apache.org>.
jacek-lewandowski commented on PR #2793:
URL: https://github.com/apache/cassandra/pull/2793#issuecomment-1786734386
https://app.circleci.com/pipelines/github/jacek-lewandowski/cassandra/1040/workflows/1020c55f-f59e-4ac6-ab0a-f589bbc1050a (j11) ⚙️
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18921-4.1: Fix DescribeStatement [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2793:
URL: https://github.com/apache/cassandra/pull/2793#discussion_r1356653028
##########
src/java/org/apache/cassandra/cql3/statements/DescribeStatement.java:
##########
@@ -132,8 +132,13 @@ public final ResultMessage execute(QueryState state, QueryOptions options, long
@Override
public ResultMessage executeLocally(QueryState state, QueryOptions options)
{
- Keyspaces keyspaces = Schema.instance.distributedAndLocalKeyspaces();
- UUID schemaVersion = Schema.instance.getVersion();
+ UUID schemaVersion;
+ Keyspaces keyspaces;
+ do
+ {
+ schemaVersion = Schema.instance.getVersionSync();
+ keyspaces = Schema.instance.distributedAndLocalKeyspaces();
Review Comment:
@jacek-lewandowski this seems interesting at first sight ... feels unnatural. Basically, we set `keyspaces` but they are not used in `while`.
does `keyspaces = ... ` have to be in that do-while too? Do I understand it right that calling `distributedAndLocalKeyspaces` will change `getVersionSync` eventually too?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-18921-4.1: Fix DescribeStatement [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2793:
URL: https://github.com/apache/cassandra/pull/2793#discussion_r1356653028
##########
src/java/org/apache/cassandra/cql3/statements/DescribeStatement.java:
##########
@@ -132,8 +132,13 @@ public final ResultMessage execute(QueryState state, QueryOptions options, long
@Override
public ResultMessage executeLocally(QueryState state, QueryOptions options)
{
- Keyspaces keyspaces = Schema.instance.distributedAndLocalKeyspaces();
- UUID schemaVersion = Schema.instance.getVersion();
+ UUID schemaVersion;
+ Keyspaces keyspaces;
+ do
+ {
+ schemaVersion = Schema.instance.getVersionSync();
+ keyspaces = Schema.instance.distributedAndLocalKeyspaces();
Review Comment:
@jacek-lewandowski this seems interesting at first sight ... feels unnatural. Basically, we set `keyspaces` but they are not used in `while`. Is that `distributedAndLocalKeyspaces` doing something which will make `getVersionSync` behave differently?
does `keyspaces = ... ` have to be in that do-while too?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org