jacek-lewandowski commented on a change in pull request #891:
URL: https://github.com/apache/cassandra/pull/891#discussion_r575987381
##########
File path: src/java/org/apache/cassandra/db/filter/ColumnFilter.java
##########
@@ -505,9 +504,16 @@ public ColumnFilter deserialize(DataInputPlus in, int
version, CFMetaData metada
}
}
- // See CASSANDRA-15833
- if (version <= MessagingService.VERSION_3014 && isFetchAll)
+ // Since nodes with and without CASSANDRA-10657 are not
distinguishable by messaging version, we need to
+ // check whether there are any nodes running pre CASSANDRA-10657
Cassandra and apply conversion to the
+ // column filter so that it is interpreted in the same way as on
the nodes without CASSANDRA-10657.
+ //
+ // see CASSANDRA-10657, CASSANDRA-15833, CASSANDRA-16415
+ if (isFetchAll && queried != null &&
Gossiper.instance.isAnyNodeOn30())
Review comment:
`anyNodeOn30` is not a flag which changes one way. It is initially set
to `false`, which means that in case of the upgrade it will change to `true`
and then eventually back to `false`. This is implemented differently on `trunk`
but I didn't want to change the behaviour of this flag here as more things
depend on it and it is just the patch release. There will be obviously small
time gap when we still get digest mismatches, but it is still better than
having them during whole upgrade. If the flag was set initially to `true`, it
would be better - initially the cluster would assume 3.0 behaviour.
For the future releases, I think we could make messaging version correspond
to version family (which means incrementing it with each new minor release) to
ensure we wouldn't be forced to base on Gossiper.
Please have a look on trunk PR because it had to be more complicated as we
need to distinguish whether we upgrade from pre 3.4 or 3.4~4.0
##########
File path: src/java/org/apache/cassandra/db/filter/ColumnFilter.java
##########
@@ -385,44 +404,24 @@ public boolean equals(Object other)
@Override
public String toString()
{
- if (isFetchAll)
- return "*";
-
- if (queried.isEmpty())
- return "";
-
- Iterator<ColumnDefinition> defs = queried.selectOrderIterator();
- if (!defs.hasNext())
- return "<none>";
-
- StringBuilder sb = new StringBuilder();
- while (defs.hasNext())
- {
- appendColumnDef(sb, defs.next());
- if (defs.hasNext())
- sb.append(", ");
+ if (isFetchAll && queried == null)
+ return "*/*";
+
+ if (isFetchAll && queried.isEmpty())
+ return "*/[]";
+
+ StringJoiner joiner = new StringJoiner(", ", isFetchAll ? "*/[" : "[",
"]");
+ Iterator<ColumnDefinition> it = queried.selectOrderIterator();
+ while (it.hasNext()) {
Review comment:
sure
----------------------------------------------------------------
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]
