4.x: Don't use overly broad SELECTs when checking schema agreement

[Lorak-mmk]

When checking schema agreement driver uses SELECT * FROM system.peers query.

It looks like the only use of columns other than schema_version is calling isPeerValid function.
As far as I can tell it is still not necessary to select everything - isPeerValid needs host_id and peer, and also calls PeerRowValidator::isValid.

Is the call to PeerRowValidator::isValid necessary for checking schema agreement? I don't know, my suspicion is that it is not.

Even if this call is necessary, there are columns that are not used even by this validation and thus can surely be not selected:

• preferred_ip
• release_version
• supported_features (this is an important one because it contains a lot of data)

Not selecting tokens would be even better (because it contains even more data than supported_features), and this column is surely not needed to check schema agreement.

[Bouncheck]

IIUC generally checking if peer is valid is necessary, but maybe not as extensively. If there is an invalid peer on the list it may lead to disagreement when it should not.
Seems like there is some background about that in https://datastax-oss.atlassian.net/browse/JAVA-2918

It sounds reasonable to limit what columns are fetched for checking schema agreement. Probably not all are needed. I'll comment further when I check the details.

[dkropachev]

IIUC generally checking if peer is valid is necessary, but maybe not as extensively. If there is an invalid peer on the list it may lead to disagreement when it should not. Seems like there is some background about that in https://datastax-oss.atlassian.net/browse/JAVA-2918

It sounds reasonable to limit what columns are fetched for checking schema agreement. Probably not all are needed. I'll comment further when I check the details.

And we can't read of record in the tokens, because scylla does not support collection_count

So, there is some small optimization possible, but in order to close it completely, we need scylla to support collection_count

[Lorak-mmk]

And we can't read of record in the tokens,

I don't understand this sentence,

Why can't we ignore tokens column?

[dkropachev]

And we can't read of record in the tokens,

I don't understand this sentence,

Why can't we ignore tokens column?

4.x driver has METADATA_ALLOW_ZERO_TOKEN_PEERS option:

java-driver/core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java

Lines 591 to 596 in c289ebf

	/**
	* Whether zero token peers should be considered valid.
	*
	* <p>Value-type: boolean
	*/
	METADATA_ALLOW_ZERO_TOKEN_PEERS("advanced.metadata.allow-zero-token-peers"),

That controls whether driver consider zero-token nodes in metadata or not, it also used in schema agreement calculation:

java-driver/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/SchemaAgreementChecker.java

Lines 192 to 216 in 75214e9

	protected boolean isPeerValid(AdminRow peerRow, Map<UUID, Node> nodes) {
	if (PeerRowValidator.isValid(
	peerRow,
	context
	.getConfig()
	.getDefaultProfile()
	.getBoolean(DefaultDriverOption.METADATA_ALLOW_ZERO_TOKEN_PEERS))) {
	UUID hostId = peerRow.getUuid("host_id");
	Node node = nodes.get(hostId);
	if (node == null) {
	LOG.warn("[{}] Unknown peer {}, excluding from schema agreement check", logPrefix, hostId);
	return false;
	} else if (node.getState() != NodeState.UP) {
	LOG.debug("[{}] Peer {} is down, excluding from schema agreement check", logPrefix, hostId);
	return false;
	}
	return true;
	} else {
	LOG.warn(
	"[{}] Found invalid system.peers row for peer: {}, excluding from schema agreement check.",
	logPrefix,
	peerRow.getInetAddress("peer"));
	return false;
	}
	}

This option is disabled by default, which means that driver have to exclude zero-token nodes from schema agreement proccess.

Without collection_count only way how driver can see if node is zero-token or not is to pull tokens from system.peers and check if it is empty or not.

[mykaul]

But how is it relevant? In other words, why would you check for zero-token nodes at every fetch from system.peers for SCHEMA agreement?