Cassgo 100 by worryg0d · Pull Request #23 · worryg0d/gocql

worryg0d · 2026-04-30T08:27:19Z

No description provided.

Copilot

Pull request overview

This PR introduces a dedicated segment codec and integrates Native Protocol v5 “segment” framing into the connection read/write paths, refactoring existing frame/segment logic and moving related tests into a new test file.

Changes:

Added segmentCodec for encoding/decoding compressed and uncompressed segments (with CRC checks).
Updated Conn startup flow to switch to segment-based reader/writer after successful handshake for proto v5+.
Moved/expanded segment-related tests out of frame_test.go into segment_codec_test.go and updated affected integration/unit tests.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`segment_codec.go`	New segment encoding/decoding implementation with header/payload checksums and compression handling.
`segment_codec_test.go`	New unit tests covering segment codec behaviors and edge cases.
`frame.go`	Removes prior segment framing helpers from frame implementation.
`frame_test.go`	Removes segment-related tests now covered by `segment_codec_test.go`.
`conn.go`	Adds segmentReader/segmentWriter and switches to segment framing for proto v5+ after startup.
`conn_test.go`	Updates tests for new recv signature and adds segmentWriter multi-frame test.
`control.go`	Uses `execInternal` in place of removed `exec`.
`cassandra_test.go`	Updates exec callsite, tweaks one session creation, and adjusts compression-related test queries/table names.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-30T08:38:35Z

+	req.resultChan <- writeResult{
+		n:   totalWritten,
+		err: flushErr,
+	}


flushBigFrameImmediately reports totalWritten (sum of encoded segment bytes written) back to the caller. contextWriter.writeContext is required to report bytes written from the caller-provided buffer (p), so this can exceed len(req.data) and also breaks callers that interpret n==0 as "not written". Return len(req.data) on success (or 0 on failure), and only use segment-level byte counts internally/for logging.

Copilot · 2026-04-30T08:38:35Z

+	for _, req := range sw.writeRequests {
+		req.resultChan <- writeResult{
+			n:   n,
+			err: nil,
+		}
+	}


flushCurrentSegment returns the segment write byte count n to every queued request. This violates the contextWriter contract (it must return bytes written from that request’s p, i.e., 0 <= n <= len(p)), and it breaks execInternal’s n==0 error handling. Return len(req.data) for each successfully queued frame (or track per-frame progress), and return 0 on segment-level failure.

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-30T10:32:06Z

+	select {
+	case <-ctx.Done():
+		return 0, ctx.Err()
+	case <-sw.quit:
+		return 0, ErrConnectionClosed
+	case result := <-resultChan:
+		return result.n, result.err
+	}


segmentWriter.writeContext can return ctx.Err() after the request has already been enqueued, but the flusher will still write that frame later. This breaks the contextWriter contract and can cause execInternal to release the stream (n==0) while the request is still sent, leading to responses on a reused stream ID. Ensure that once a request is queued it either (a) is guaranteed not to be written when returning ctx.Err(), or (b) writeContext waits for the queued write result (ignoring ctx) similar to writeCoalescer, or (c) supports canceling/removing the queued request before it is flushed.

Copilot · 2026-04-30T10:32:06Z

+			return
+		case req := <-sw.writeCh:
+			frame := req.data
+			if len(frame) > maxSegmentPayloadSize {


When a frame larger than maxSegmentPayloadSize arrives, runFlusher writes it immediately via flushBigFrameImmediately without first flushing any already-queued smaller frames. That can reorder writes on the connection (later big frame sent before earlier queued frames). Flush the current segment (if non-empty) and reset state before writing the big frame to preserve ordering.

Suggested change

if len(frame) > maxSegmentPayloadSize {

if len(frame) > maxSegmentPayloadSize {

if len(sw.writeRequests) > 0 {

sw.flushCurrentSegment()

sw.reset()

running = false

}

Copilot · 2026-04-30T10:32:07Z

 	return fmt.Errorf("gocql: cluster schema versions not consistent: %+v", schemas)
 }

+// segmentWriter allows batching multiple frames into a signle segment before flushing them to the connection.


Typo in comment: "signle" should be "single".

Suggested change

// segmentWriter allows batching multiple frames into a signle segment before flushing them to the connection.

// segmentWriter allows batching multiple frames into a single segment before flushing them to the connection.

Copilot · 2026-04-30T10:32:07Z

+	// synced by doneReadCh
+	resultBuf := make([]byte, 0, 128)
+
+	go func() {
+		defer close(doneReadCh)
+		buf := make([]byte, 128)
+		n, err := server.Read(buf)
+		if err != nil && err != io.EOF {
+			t.Errorf("Failed to read segment: %v", err)
+			return
+		}
+		// Expected to read only a single segment with two frames inside
+		segmentCodec := newSegmentCodec(nil)
+		body, isSelfContained, err := segmentCodec.decode(bytes.NewReader(buf[:n]))
+		require.NoError(t, err)
+		require.True(t, isSelfContained)
+		resultBuf = append(resultBuf, body...)
+	}()
+
+	go func() {
+		sw.writeContext(context.Background(), []byte("one"))
+	}()
+
+	go func() {
+		sw.writeContext(context.Background(), []byte("two"))
+	}()
+
+	select {
+	case <-doneReadCh:


TestSegmentWriter_MultipleFrames reads from a real TCP connection with a single server.Read call and assumes it received an entire segment. TCP reads can be partial, making this test flaky. Consider decoding directly from the conn via segmentCodec.decode(server) (which uses io.ReadFull internally) or reading in a loop until the full segment is available; also capture/require errors returned by sw.writeContext in the writer goroutines so failures don't get silently ignored.

Suggested change

// synced by doneReadCh

resultBuf := make([]byte, 0, 128)

go func() {

defer close(doneReadCh)

buf := make([]byte, 128)

n, err := server.Read(buf)

if err != nil && err != io.EOF {

t.Errorf("Failed to read segment: %v", err)

return

}

// Expected to read only a single segment with two frames inside

segmentCodec := newSegmentCodec(nil)

body, isSelfContained, err := segmentCodec.decode(bytes.NewReader(buf[:n]))

require.NoError(t, err)

require.True(t, isSelfContained)

resultBuf = append(resultBuf, body...)

}()

go func() {

sw.writeContext(context.Background(), []byte("one"))

}()

go func() {

sw.writeContext(context.Background(), []byte("two"))

}()

select {

case <-doneReadCh:

writeErrCh := make(chan error, 2)

// synced by doneReadCh

resultBuf := make([]byte, 0, 128)

go func() {

defer close(doneReadCh)

// Expected to read only a single segment with two frames inside.

// Decode directly from the connection so partial reads are handled correctly.

segmentCodec := newSegmentCodec(nil)

body, isSelfContained, err := segmentCodec.decode(server)

require.NoError(t, err)

require.True(t, isSelfContained)

resultBuf = append(resultBuf, body...)

}()

go func() {

writeErrCh <- sw.writeContext(context.Background(), []byte("one"))

}()

go func() {

writeErrCh <- sw.writeContext(context.Background(), []byte("two"))

}()

select {

case <-doneReadCh:

require.NoError(t, <-writeErrCh)

require.NoError(t, <-writeErrCh)

Copilot · 2026-04-30T10:32:07Z

+// segment_codec.go
+
+package gocql
+
+import (


New Go source files in this repo typically include the ASF license header (and the historical Gocql BSD notice) at the top (e.g., frame.go:1-23, cluster.go:1-23). segment_codec.go currently lacks this header; please add the standard license block to keep licensing consistent.

Previously, the driver encoded each individual frame in a single segment despite segments ability to hold multiple frames. This patch introduces a mechanism that allows driver collect multiple frames before encoding them as a batch. To achieve this, segmentWriter was introduced. Patch by Bohdan Siryk; reviewed by TBD for CASSGO-100

Prevent write hang on segmentWriter.writeContext call when writer is closed. Prevent stale timer tick when current segment is about to be flushed because new request doesn't fit current segment

… slice of bytes

worryg0d requested a review from Copilot April 30, 2026 08:27

Copilot started reviewing on behalf of worryg0d April 30, 2026 08:28 View session

Copilot AI reviewed Apr 30, 2026

View reviewed changes

worryg0d requested a review from Copilot April 30, 2026 10:24

Copilot started reviewing on behalf of worryg0d April 30, 2026 10:25 View session

Copilot AI reviewed Apr 30, 2026

View reviewed changes

worryg0d force-pushed the cassgo-100 branch from e16168a to 13ce00c Compare April 30, 2026 11:31

worryg0d added 13 commits May 20, 2026 13:25

address review typos fixes

774ddb6

flush current segment before flushing big frame

6b317c7

removed unsusd global err and return err instead of panic

252360c

resuable read buffers for codec and godoc

4add732

more unit tests for segmentWriter

3d25eec

segmentReader unit tests

6d74ccb

segment codec benchmarks

f95fd6f

fixed data race in test helper

2d0d3ff

Prevent write hang and stale timer tick

f641b6e

Prevent write hang on segmentWriter.writeContext call when writer is closed. Prevent stale timer tick when current segment is about to be flushed because new request doesn't fit current segment

segmentWriter benchmark

963774c

refactor segment codec to take [][]byte payload instead of contiguous…

a830a4c

… slice of bytes

segmentWriter: use writev on large frame path

246c507

reusable buffers on segmentCodec and segmentWriter

4b0724e

worryg0d force-pushed the cassgo-100 branch from 2e7194b to 4b0724e Compare June 26, 2026 14:41

worryg0d closed this Jul 21, 2026

Navigation Menu

Cassgo 100 - #23

Cassgo 100#23
worryg0d wants to merge 14 commits into
masterfrom
cassgo-100

worryg0d commented Apr 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-			if len(frame) > maxSegmentPayloadSize {
+			if len(frame) > maxSegmentPayloadSize {
+				if len(sw.writeRequests) > 0 {
+					sw.flushCurrentSegment()
+					sw.reset()
+					running = false
+				}

	// segmentWriter allows batching multiple frames into a signle segment before flushing them to the connection.
	// segmentWriter allows batching multiple frames into a single segment before flushing them to the connection.

Conversation

worryg0d commented Apr 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants