4.x: Do not expose NodeShardingInfo via Node

[scrat98]

With ScyllaDB Tablets enabled by default, it is no longer possible to use a consistent algorithm to map a Token to a ShardId. This is because a Tablet (which holds a set of tokens) can migrate between shards within the same node or even across nodes for load balancing.

CqlRequestHandler correctly relies on the TabletMap(getShardFromTabletMap) https://github.com/scylladb/java-driver/blob/scylla-4.x/core/src/main/java/com/datastax/oss/driver/internal/core/cql/CqlRequestHandler.java#L369

But obtaining shardId via Node still relies on the calculation: https://github.com/scylladb/java-driver/blob/scylla-4.x/core/src/main/java/com/datastax/oss/driver/internal/core/protocol/ShardingInfo.java#L66
And users can still use Node.getShardingInfo().getShardId(token) for grouping the statements for example, like here #232

Even with Tablets disabled, mapping a Token to a ShardId is not feasible because there is no predictable relationship between ShardId and a specific CPU. Each node manages its shards independently (and that mappings may be changes after restart, for example) and does not rely on consistent hashing of token ranges. https://opensource.docs.scylladb.com/stable/kb/map-cpu.html

To avoid users confusion and mistakes I would suggest deprecate this method and mark it as for removal. In the driver source code everything uses TabletMap already and not rely on the Node.getShardId

Note: According to the protocol documentation "biased-token-round-robin" algorithm should still work, but I'm not sure if it's updated or not, because then I can't understand how it should work with Tablets. Therefore, I'm sorry in advance if my knowledge about scylla is wrong and all the statements above as well https://github.com/scylladb/scylladb/blob/master/docs/dev/protocol-extensions.md#intranode-sharding

[Lorak-mmk]

Even with Tablets disabled, mapping a Token to a ShardId is not feasible because there is no predictable relationship between ShardId and a specific CPU. Each node manages its shards independently (and that mappings may be changes after restart, for example) and does not rely on consistent hashing of token ranges. https://opensource.docs.scylladb.com/stable/kb/map-cpu.html

That's not really correct. Mapping from Token to ShardId is feasible, and useful. You don't know how those shard ids correspond to CPU cores, but that would not be useful for anything.
Usually you use shard id calculation to e.g. batch requests by target.

To avoid users confusion and mistakes I would suggest deprecate this method and mark it as for removal. In the driver source code everything uses TabletMap already and not rely on the Node.getShardId

@Bouncheck I remember that on one of our team calls we discussed similar topic (metadata APIs tablet compatibility). IIRC Java Driver does have APIs that work with both vnodes and tablets, right?
If so, it is probably a good idea to deprecate those APIs that only work with one of the methods with an appropriate warning. If there are no APIs that work with both, then we need to fix that.

[scrat98]

@Lorak-mmk

That's not really correct. Mapping from Token to ShardId is feasible, and useful. You don't know how those shard ids correspond to CPU cores, but that would not be useful for anything.
Usually you use shard id calculation to e.g. batch requests by target.

Thanks, so it means:

• for Tablets enabled use case - TabletMap is used
• for Tablets disabled use case - "biased-token-round-robin" calculation

right?

[Lorak-mmk]

Yes, but this is not something that a user should really care about. The driver should expose interfaces that work correctly for both tablet and vnode keyspaces.

[Bouncheck]

@Bouncheck I remember that on one of our team calls we discussed similar topic (metadata APIs tablet compatibility). IIRC Java Driver does have APIs that work with both vnodes and tablets, right? If so, it is probably a good idea to deprecate those APIs that only work with one of the methods with an appropriate warning. If there are no APIs that work with both, then we need to fix that.

API is different in 3.x and 4.x.
In 3.x there are public methods like Metadata#getReplicasList(keyspace, table, partitioner, key) which should work with both.
In 4.x Metadata itself there is no similar public API, but it is possible to get the tokenmap or tabletmap and query them individually. So in regards to that 4.x could be improved, since there is no unified API.
There technically is getReplicas() counterpart but its not public API and its part of BasicLoadBalancingPolicy, not even LoadBalancingPolicy interface.

[scrat98]

I think it make sense to maybe extend LoadBalancingPolicy interface. Because the only reason why people wanted to know about shardId is the same why people wanted to know about node - to make a batch by node/shard with multiple partition keys to save network OPS. Currently using newQueryPlan we can obtain replica nodes (not exactly, but it's okay for such use cases), so maybe we can add similar method which will return not queue of Nodes, but queue of Shards (nodeId + shardId)