yifan-c commented on a change in pull request #892:
URL: https://github.com/apache/cassandra/pull/892#discussion_r579383623
##########
File path: src/java/org/apache/cassandra/gms/Gossiper.java
##########
@@ -2130,7 +2136,12 @@ public boolean waitForSchemaAgreement(long maxWait,
TimeUnit unit, BooleanSuppli
public boolean haveMajorVersion3Nodes()
{
- return haveMajorVersion3NodesMemoized.get();
+ return isUpgradingFromVersionLowerThan(4, 0);
+ }
+
+ public boolean isUpgradingFromVersionLowerThan(int major, int minor) {
+ CassandraVersion v = upgradeFromVersionMemoized.get();
+ return v != null && v.isLowerThan(major, minor);
Review comment:
Regarding upgrade detection, I can think of 2 approaches. Both have
their good and bad.
- Something similar to `SystemKeyspaceMigrator40`, that check if the new
table introduced in 40 exist at local. The good: good UX. The bad: what if the
instance has migrated the tables, but failed to start. The operator restarts
the instance during cluster upgrade. Now the instance is unable to detect.
- Accept a explicit property, say `cassandra.isUpgrade: Boolean`. The good:
gives the operator more control. The bad: another property to handle (in some
automation/manual process), and need to unset the property later.
Regarding returning `NULL_VERSION`, I just thought of a failure scenario...
The cluster is upgrading from `3.11` to `4.0`. When `NULL_VERSION` is
returned, the 4.0 instance assumes the earliest behavior, which is `3.4-`, and
there is a chance for digest mismatch. We can update the property to
`cassandra.upgradeFrom: String = CassandraVersion`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
