[#591] Merge upstream commit `63b6d78`: "CASSGO 1 Support for Native Protocol 5" #593

nikagra · 2025-10-17T18:13:07Z

Merging upstream commit 63b6d78:

Native Protocol 5 was introduced with the release of C* 4.0. This PR provides full support for a newer version including new format frames (segments), and new fields for QUERY, BATCH, and EXECUTE messages. Also, this PR brings changes to the Compressor interface to follow an append-like design. One more thing, it bumps Go version to the newer 1.19.

Patch by Bohdan Siryk; Reviewed by João Reis, James Hartig for CASSGO-1 CASSGO-30

These changes introduce functionality necessary to implement SCYLLA_USE_METADATA_ID protocol extension (see #527).

Extra changes:

Extracting CRC functionality into a package.
Removing headSize from framer.
Adding validation to ReadTimeout value in ClusterConfig
Adding .vscode to .gitignore.

control.go

internal/crc/crc.go

frame.go

session.go

mykaul · 2025-10-18T07:32:14Z

Native Protocol 5 was introduced with the release of C* 4.0. This PR provides full support for a newer version including new format frames (segments), and new fields for QUERY, BATCH, and EXECUTE messages. Also, this PR brings changes to the Compressor interface to follow an append-like design. One more thing, it bumps Go version to the newer 1.19.

What's the value of the v5 protocol?

dkropachev · 2025-10-18T10:57:35Z

Native Protocol 5 was introduced with the release of C* 4.0. This PR provides full support for a newer version including new format frames (segments), and new fields for QUERY, BATCH, and EXECUTE messages. Also, this PR brings changes to the Compressor interface to follow an append-like design. One more thing, it bumps Go version to the newer 1.19.

What's the value of the v5 protocol?

It will allow to test modern Cassandra side by side, bring proper metadata handling, which we will use in next pr.
And I hope, eventually we are going to get cql5 too.

mykaul · 2025-10-18T15:32:03Z

Native Protocol 5 was introduced with the release of C* 4.0. This PR provides full support for a newer version including new format frames (segments), and new fields for QUERY, BATCH, and EXECUTE messages. Also, this PR brings changes to the Compressor interface to follow an append-like design. One more thing, it bumps Go version to the newer 1.19.

What's the value of the v5 protocol?

It will allow to test modern Cassandra side by side, bring proper metadata handling, which we will use in next pr. And I hope, eventually we are going to get cql5 too.

For proper metada handling we need this change to the wire protocol? v5 doesn't seem to be providing huge value, maybe I'm missing something. Modern cassandra isn't exciting. CQL5 - I'm not sure it's on our todo list just yet.

dkropachev · 2025-10-20T17:51:17Z

Native Protocol 5 was introduced with the release of C* 4.0. This PR provides full support for a newer version including new format frames (segments), and new fields for QUERY, BATCH, and EXECUTE messages. Also, this PR brings changes to the Compressor interface to follow an append-like design. One more thing, it bumps Go version to the newer 1.19.

What's the value of the v5 protocol?

It will allow to test modern Cassandra side by side, bring proper metadata handling, which we will use in next pr. And I hope, eventually we are going to get cql5 too.

For proper metada handling we need this change to the wire protocol? v5 doesn't seem to be providing huge value, maybe I'm missing something. Modern cassandra isn't exciting. CQL5 - I'm not sure it's on our todo list just yet.

We don't need it, but if we don't bring CQL5 now, later we will have to deal with rewiring metadata updates when CQL5 going to get it, which will make it way harder, out of experience with other drivers I can tell that bringing CQL5 first and then implementing metadata update feature way easier than otherway around.

Native Protocol 5 was introduced with the release of C* 4.0. This PR provides full support for a newer version including new format frames (segments), and new fields for QUERY, BATCH, and EXECUTE messages. Also, this PR brings changes to the Compressor interface to follow an append-like design. One more thing, it bumps Go version to the newer 1.19. Patch by Bohdan Siryk; Reviewed by João Reis, James Hartig for CASSGO-1 CASSGO-30

…y to internal package. Integration tests config changes. Reverting unwanted merge changes. Detailed summary: - Moving CRC functionality to internal package. - Update test cluster hosts to match CCM-created cluster - Update flagCluster default from 127.0.0.1 to 127.0.2.1,127.0.2.2,127.0.2.3 to align with the 3-node cluster configuration created by CCM. Fixes connection refused errors in integration tests. - Adding validation to `ReadTimeout` value in `ClusterConfig` - Removing `headSize` from `framer`. - Adding `.vscode` to `.gitignore`.

dkropachev · 2025-10-31T15:29:02Z

control.go

 	hosts = shuffleHosts(hosts)

 	connCfg := *c.session.connCfg
 	connCfg.ProtoVersion = protoVersion4 // TODO: define maxProtocol


Please set it to 5

it is still protoVersion4

dkropachev · 2025-10-31T15:35:03Z

cassandra_test.go

+		text string
+	)
+
+	q := session.Query("SELECT * FROM gocql_query_keyspace_override_test.query_keyspace").


Please remove ks from the query.

it is still there.

dkropachev · 2025-10-31T15:35:15Z

cassandra_test.go

+	id = 0
+	text = ""
+
+	q = session.Query("SELECT * FROM gocql_query_keyspace_override_test.query_keyspace").


Please remove ks from the query.

dkropachev · 2025-10-31T15:51:06Z

common_test.go


 var (
-	flagCluster       = flag.String("cluster", "127.0.0.1", "a comma-separated list of host:port tuples")
+	flagCluster       = flag.String("cluster", "127.0.2.1,127.0.2.2,127.0.2.3", "a comma-separated list of host:port tuples")


Please roll it back

dkropachev · 2025-10-31T19:03:31Z

conn.go

+	if c.r.GetTimeout() > 0 {
+		c.r.SetReadDeadline(time.Time{})
 	}


This one does not make any sense to have it here, it will be overridden on next .Read call on the reader, please drop it

Let's do the following instead, on first io.ReadFull(r, p[:1]) in the readHeader(r io.Reader, p []byte) (head frameHeader, err error) let's handle read timeout and wrap/or return an error that signals that it is an expected situation and handle it gracefully on serve(ctx context.Context).

The problem is the following, if there is no traffic serve(ctx context.Context) is still calling readHeader, in which situation it is ok to get timeout on reading begging of the frame.

dkropachev · 2025-10-31T19:14:10Z

conn.go

+
+// ConnReader is like net.Conn but also allows to set timeout duration.
+type ConnReader interface {
+	net.Conn


Please remove net.Conn and add only methods that are used.

dkropachev · 2025-10-31T19:16:23Z

conn.go

+func (c *connReader) LocalAddr() net.Addr {
+	return c.conn.LocalAddr()
+}


It is not used, drop it

dkropachev · 2025-10-31T19:16:28Z

conn.go

+	return c.conn.RemoteAddr()
+}
+
+func (c *connReader) SetDeadline(t time.Time) error {


It is not used, drop it

dkropachev · 2025-10-31T19:16:42Z

conn.go

+func (c *connReader) SetReadDeadline(t time.Time) error {
+	return c.conn.SetReadDeadline(t)
+}


It should not be used, please drop it.

dkropachev · 2025-10-31T19:17:35Z

conn.go

+func (c *connReader) SetWriteDeadline(t time.Time) error {
+	return c.conn.SetWriteDeadline(t)
+}


SetWriteDeadline on a connReader ? Please drop it.

…nhancements. Refactor keyspace handling in `TestQuery_SetKeyspace`

dkropachev

Please go over all the comments, make sure they are fixed.

dkropachev · 2025-11-05T01:55:37Z

control.go

 	hosts = shuffleHosts(hosts)

 	connCfg := *c.session.connCfg
 	connCfg.ProtoVersion = protoVersion4 // TODO: define maxProtocol


it is still protoVersion4

dkropachev · 2025-11-05T01:56:31Z

cassandra_test.go

+		text string
+	)
+
+	q := session.Query("SELECT * FROM gocql_query_keyspace_override_test.query_keyspace").


it is still there.

nikagra requested a review from dkropachev October 17, 2025 18:13

nikagra force-pushed the merge-upstream-changes branch from d894ba4 to b5ee23f Compare October 17, 2025 18:18

dkropachev requested changes Oct 17, 2025

View reviewed changes

control.go Outdated Show resolved Hide resolved

internal/crc/crc.go Show resolved Hide resolved

frame.go Outdated Show resolved Hide resolved

frame.go Outdated Show resolved Hide resolved

session.go Outdated Show resolved Hide resolved

nikagra force-pushed the merge-upstream-changes branch 2 times, most recently from 80e1137 to 18a0652 Compare October 27, 2025 18:20

nikagra force-pushed the merge-upstream-changes branch from e7022a5 to 5a76087 Compare October 29, 2025 16:56

nikagra changed the title ~~CASSGO 1 Support for Native Protocol 5~~ Merge upstream commit 63b6d78: "CASSGO 1 Support for Native Protocol 5" Oct 30, 2025

nikagra force-pushed the merge-upstream-changes branch from 5a76087 to a2cb754 Compare October 30, 2025 12:35

nikagra marked this pull request as ready for review October 30, 2025 12:36

nikagra requested review from dkropachev and sylwiaszunejko October 30, 2025 12:36

nikagra changed the title ~~Merge upstream commit 63b6d78: "CASSGO 1 Support for Native Protocol 5"~~ [#591] Merge upstream commit 63b6d78: "CASSGO 1 Support for Native Protocol 5" Oct 30, 2025

dkropachev reviewed Oct 31, 2025

View reviewed changes

[scylladb#591] Cleaning up interfaces. Header read timeout handling e…

5aa5745

…nhancements. Refactor keyspace handling in `TestQuery_SetKeyspace`

nikagra requested a review from dkropachev November 4, 2025 17:32

dkropachev requested changes Nov 5, 2025

View reviewed changes

[#591] Merge upstream commit 63b6d78: "CASSGO 1 Support for Native Protocol 5" #593

Are you sure you want to change the base?

[#591] Merge upstream commit 63b6d78: "CASSGO 1 Support for Native Protocol 5" #593

Uh oh!

Conversation

nikagra commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mykaul commented Oct 18, 2025

Uh oh!

dkropachev commented Oct 18, 2025

Uh oh!

mykaul commented Oct 18, 2025

Uh oh!

dkropachev commented Oct 20, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkropachev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[#591] Merge upstream commit `63b6d78`: "CASSGO 1 Support for Native Protocol 5" #593

[#591] Merge upstream commit `63b6d78`: "CASSGO 1 Support for Native Protocol 5" #593

nikagra commented Oct 17, 2025 •

edited

Loading