tejalchak commented on code in PR #2094:
URL:
https://github.com/apache/cassandra-java-driver/pull/2094#discussion_r3608868492
##########
core/src/main/java/com/datastax/oss/driver/internal/core/cql/CqlRequestHandler.java:
##########
@@ -300,14 +301,30 @@ private void sendRequest(
currentExecutionIndex,
retryCount,
scheduleNextExecution,
- logPrefixJoiner.join(this.sessionName, nodeRequestId,
currentExecutionIndex));
+ logPrefixJoiner.join(this.sessionName, nodeRequestId,
currentExecutionIndex),
+ resultMetadataSnapshot(statement));
Message message = Conversions.toMessage(statement, executionProfile,
context);
Review Comment:
I was looking at the race condition window here and wanted to get a second
pair of eyes ...
While the callback now safely uses the snapshot, toMessage still reads the
live metadata state. I'm wondering if a concurrent schema update between
capturing the snapshot and the toMessage execution could cause a mismatch
(where the request sends the new metadata ID, but the callback decodes using
the old snapshot). Should toMessage accept the snapshot to close any chance of
discrepancy?
Since ConversionsMetadataRaceTest calls getResultDefinitions directly rather
than driving through toMessage, so it might not catch this without some change.
##########
core/src/main/java/com/datastax/oss/driver/internal/core/cql/DefaultPreparedStatement.java:
##########
@@ -210,13 +220,21 @@ public RepreparePayload getRepreparePayload() {
return this.repreparePayload;
}
- private static class ResultMetadata {
- private ByteBuffer resultMetadataId;
- private ColumnDefinitions resultSetDefinitions;
+ static class ResultMetadata {
+ private final ByteBuffer resultMetadataId;
+ private final ColumnDefinitions resultSetDefinitions;
private ResultMetadata(ByteBuffer resultMetadataId, ColumnDefinitions
resultSetDefinitions) {
this.resultMetadataId = resultMetadataId;
this.resultSetDefinitions = resultSetDefinitions;
}
+
+ ByteBuffer getResultMetadataId() {
+ return resultMetadataId;
+ }
+
+ ColumnDefinitions getResultSetDefinitions() {
+ return resultSetDefinitions;
+ }
Review Comment:
A minor thought: if this branch is strictly Java 17+, we could consider
using a record to avoid the boilerplate code
`static record ResultMetadata(ByteBuffer resultMetadataId, ColumnDefinitions
resultSetDefinitions) {}`
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
