CASSGO-100 Support frames batching into segment by worryg0d · Pull Request #1946 · apache/cassandra-gocql-driver

worryg0d · 2026-04-24T12:06:51Z

TBD

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

worryg0d · 2026-05-04T07:31:14Z

I ended up with a duplicate of the current writeCoalescer behavior, but with segment-encoding specifics. Changes to the framing format are rare, so I think it is reasonable not to spend time implementing a flexible solution that would allow the application of encoders in flight. But I'm open to discussion here.

Also, I introduced segmentReader wrapper around the ConnReader interface, which bundles frame-related logic in recv() and segment-related logic.

Also, I added a segmentCodec that encapsulates segment encoding/decoding, but I do not really insist on merging it, so I am open to discussion.

joao-r-reis

Looking good overall but left a few comments. It would be nice to run benchmarks to compare the performance vs trunk before merging this.

I have some code I used to do a basic comparison between gocql v1 and v2 maybe it can be useful to you: https://github.com/joao-r-reis/gocql-benchmarks/tree/testv3

joao-r-reis · 2026-05-19T16:59:12Z

+// Flushes the current segment and writes the results to the result listeners.
+// Should be called before resetting the segment writer.
+func (sw *segmentWriter) flushCurrentSegment() {
+	framesBuf := make([]byte, 0, sw.totalFramesLength)


Can we not just reuse a single instance of bytes.Buffer? If I'm not mistaken this slice is only needed until encodeAndWrite below

You mean make part of the segment writer? I'm concerned about having a big backed slice for each conn. It's around 128 kb

It will potentially grow up to that size but if you're that concerned about it you can just resize it after a write forces the buffer to grow that large.

Since v4 code doesn't reuse a buffer either let's keep it this way and let's create a low priority ticket to prototype and analyze the performance impact of making both write coalescers reuse a buffer.

joao-r-reis · 2026-05-19T17:16:10Z

+		// var verr net.Error
+		// if errors.As(err, &verr) {
+		// 	return nil, false, verr
+		// }


This TODO needs to be addressed. Personally I think it's fine to close the connection when the driver fails to encode/decode something here because it's either an error on the decode/encode code and most likely a driver bug or it's a connection error. A connection error should lead to connection closure and the encode/decode error should never happen so I don't see a lot of benefit in trying to handle it.

I'm thinking about timeouts, actually. Currently, the driver ignores the timeout when trying to read the frame header from the conn. With this patch, the driver will also ignore reading both the segment header and body, which differs from the previous driver behavior.

I'm unsure if this is good

Are you sure it ignores the timeout? Doesn't the Read loop on connReader return the error immediately instead of retrying when it's a read timeout?

cassandra-gocql-driver/conn.go

Lines 904 to 925 in c65c762

func (c *connReader) Read(p []byte) (n int, err error) {

const maxAttempts = 5

for i := 0; i < maxAttempts; i++ {

var nn int

if c.timeout > 0 {

c.conn.SetReadDeadline(time.Now().Add(c.timeout))

}

nn, err = io.ReadFull(c.r, p[n:])

n += nn

if err == nil {

break

}

if verr, ok := err.(net.Error); !ok || !verr.Temporary() {

break

}

}

return

}

In case of net.Error it do not break if I'm not missing anything.

https://pkg.go.dev/net#Error tells us whether it is timeout or not. Actually, probably worth updating the code to rely on net.Error.Timeout() intstead of Temporary() as suggested

Yeah I misread it you're right. I think this will sort itself out when we implement the timeout fix which should go in (and get released) before this PR since this is not a bug fix. After the timeout fix is merged then the timeout will not even exist here.

Yeah, if we get rid of the read socket timeouts it won't be a problem at all

worryg0d · 2026-05-19T18:21:57Z

Thanks for the review. I'm going to address it in a couple of days

worryg0d · 2026-05-28T09:06:37Z

I have run a couple of benchmarks both with and without compression for proto 5. I'm genially surprised how significant improvement for compression path. Base is trunk. Candidate is changes in this PR.
Benchmark setup:

Ubuntu 24.04.3 LTS x86_64 CPU i7-13700H 32 Gb RAM
Options:
warmup cycles: 10000
cycles: 1000000
rows: 10000000
goroutines: 64
protocol version: 5

INSERT (base vs candidate, %)

percentile	base ms	candidate ms	delta
p50	0.659	0.679	+3.03%
p90	2.234	2.298	+2.86%
p95	3.145	3.180	+1.11%
p99	5.226	5.194	-0.61%

SELECT (base vs candidate, %)

percentile	base ms	candidate ms	delta
p50	0.652	0.670	+2.76%
p90	2.232	2.307	+3.36%
p95	3.197	3.225	+0.88%
p99	5.410	5.328	-1.52%

INSERT (compression) (base vs candidate, %)

percentile	base ms	candidate ms	delta
p50	1.933	0.922	-52.30%
p90	5.205	2.485	-52.26%
p95	6.836	3.294	-51.82%
p99	10.165	5.216	-48.68%

SELECT (compression) (base vs candidate, %)

percentile	base ms	candidate ms	delta
p50	2.017	0.902	-55.28%
p90	5.474	2.467	-54.93%
p95	7.246	3.282	-54.71%
p99	10.684	5.204	-51.29%

worryg0d · 2026-05-28T09:28:11Z

Added also v4 benchmarks:

Non-compression

INSERT

percentile	proto v4 trunk ms	proto v5 trunk ms	proto v5 cassgo100 ms	cassgo100 vs v5 trunk	cassgo100 vs v4 trunk
p50	0.628	0.659	0.679	+3.03%	+8.12%
p90	2.113	2.234	2.298	+2.86%	+8.75%
p95	3.052	3.145	3.180	+1.11%	+4.19%
p99	5.147	5.226	5.194	-0.61%	+0.91%

SELECT

percentile	proto v4 trunk ms	proto v5 trunk ms	proto v5 cassgo100 ms	cassgo100 vs v5 trunk	cassgo100 vs v4 trunk
p50	0.623	0.652	0.670	+2.76%	+7.54%
p90	2.099	2.232	2.307	+3.36%	+9.91%
p95	3.092	3.197	3.225	+0.88%	+4.30%
p99	5.308	5.410	5.328	-1.52%	+0.38%

Compression

INSERT

percentile	proto v4 trunk ms	proto v5 trunk ms	proto v5 cassgo100 ms	cassgo100 vs v5 trunk	cassgo100 vs v4 trunk
p50	1.887	1.933	0.922	-52.30%	-51.14%
p90	4.904	5.205	2.485	-52.26%	-49.33%
p95	6.490	6.836	3.294	-51.82%	-49.24%
p99	9.624	10.165	5.216	-48.68%	-45.80%

SELECT

percentile	proto v4 trunk ms	proto v5 trunk ms	proto v5 cassgo100 ms	cassgo100 vs v5 trunk	cassgo100 vs v4 trunk
p50	1.913	2.017	0.902	-55.28%	-52.85%
p90	5.178	5.474	2.467	-54.93%	-52.36%
p95	6.902	7.246	3.282	-54.71%	-52.45%
p99	10.164	10.684	5.204	-51.29%	-48.80%

Memory:

Non-compression (proto v4 trunk vs proto v5 trunk vs proto v5 cassgo100)

metric	proto v4 trunk	proto v5 trunk	proto v5 cassgo100	cassgo100 vs v5 trunk	cassgo100 vs v4 trunk
avg MB	8.710	10.205	9.314	-8.73%	+6.94%
min MB	5.488	6.093	5.714	-6.22%	+4.11%
max MB	13.054	19.078	15.520	-18.65%	+18.89%

Compression (proto v4 trunk vs proto v5 trunk vs proto v5 cassgo100)

metric	proto v4 trunk	proto v5 trunk	proto v5 cassgo100	cassgo100 vs v5 trunk	cassgo100 vs v4 trunk
avg MB	18.889	20.235	10.761	-46.82%	-43.05%
min MB	7.369	7.738	6.033	-22.03%	-18.14%
max MB	78.335	57.707	19.479	-66.24%	-75.13%

worryg0d · 2026-05-28T16:04:44Z

Re-run benchmarks one more time

vs columns: percent change for cassgo-100 proto5 vs each v2.1.1 baseline. Latency: positive = slower, negative = faster. Memory: positive = higher usage, negative = lower usage.

No compression (LZ4 off)

Latency

INSERT

percentile	v2.1.1 proto4	v2.1.1 proto5	cassgo-100 proto5	vs v2.1.1 proto4	vs v2.1.1 proto5
p50	0.601ms	0.680ms	0.705ms	+17.3%	+3.7%
p90	2.115ms	2.365ms	2.482ms	+17.4%	+4.9%
p95	3.067ms	3.382ms	3.432ms	+11.9%	+1.5%
p99	5.297ms	5.745ms	5.617ms	+6.0%	-2.2%

SELECT

percentile	v2.1.1 proto4	v2.1.1 proto5	cassgo-100 proto5	vs v2.1.1 proto4	vs v2.1.1 proto5
p50	0.597ms	0.674ms	0.694ms	+16.2%	+3.0%
p90	2.086ms	2.365ms	2.495ms	+19.6%	+5.5%
p95	3.078ms	3.467ms	3.480ms	+13.1%	+0.4%
p99	5.395ms	5.943ms	5.772ms	+7.0%	-2.9%

Memory (Go heap)

Heap allocation

metric	v2.1.1 proto4	v2.1.1 proto5	cassgo-100 proto5	vs v2.1.1 proto4	vs v2.1.1 proto5
avg	9.795 MB	9.318 MB	9.950 MB	+1.6%	+6.8%
min	5.462 MB	5.927 MB	5.504 MB	+0.8%	-7.1%
max	16.638 MB	15.660 MB	14.863 MB	-10.7%	-5.1%

With compression (LZ4 on)

Latency

INSERT

percentile	v2.1.1 proto4	v2.1.1 proto5	cassgo-100 proto5	vs v2.1.1 proto4	vs v2.1.1 proto5
p50	1.941ms	2.032ms	0.971ms	-50.0%	-52.2%
p90	5.063ms	5.527ms	2.749ms	-45.7%	-50.3%
p95	6.653ms	7.269ms	3.717ms	-44.1%	-48.9%
p99	9.891ms	10.735ms	5.974ms	-39.6%	-44.4%

SELECT

percentile	v2.1.1 proto4	v2.1.1 proto5	cassgo-100 proto5	vs v2.1.1 proto4	vs v2.1.1 proto5
p50	1.968ms	2.113ms	0.950ms	-51.7%	-55.0%
p90	5.303ms	5.841ms	2.732ms	-48.5%	-53.2%
p95	7.055ms	7.721ms	3.687ms	-47.7%	-52.2%
p99	10.454ms	11.273ms	5.970ms	-42.9%	-47.0%

Memory (Go heap)

Heap allocation

metric	v2.1.1 proto4	v2.1.1 proto5	cassgo-100 proto5	vs v2.1.1 proto4	vs v2.1.1 proto5
avg	20.133 MB	16.577 MB	11.954 MB	-40.6%	-27.9%
min	7.948 MB	7.670 MB	6.362 MB	-20.0%	-17.1%
max	62.891 MB	56.450 MB	20.852 MB	-66.8%	-63.1%

joao-r-reis · 2026-05-28T16:06:53Z

Yeah I remember running some benchmarks to compare v2 with v1 and being a bit suprised that compression was so slow but I'm still surprised to see performance gains of that order now.

Do you have any idea why there seems to be a hit on v4 non compression scenarios?

worryg0d · 2026-05-28T16:17:42Z

Do you have any idea why there seems to be a hit on v4 non compression scenarios

I'm suspecting vectored IO in writeCoalscer which segmentWriter doesn't use. The segment writer allocates more memory when concatenating frames before passing them to the encoder, and encoder also allocates memory during payload encoding/decoding, but it is also true for the uncompressed path...

What actually changed is that the driver now does not compress each frame, but instead each segment, and each segment could have multiple frames

worryg0d · 2026-05-28T16:32:16Z

I added memory usage for comparison to the last benchmark report

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

joao-r-reis · 2026-06-18T12:37:44Z

is this still WIP or is it ready?

worryg0d · 2026-06-18T16:51:54Z

I did a lot of testing of different approaches to reduce performance impact on the uncompressed path. Right now, the best thing is to change the segmentCodec.encode API to take [][]bytes instead of a temporarily allocated buffer that holds all frames concatenated -1 allocations. But I didn't test it yet through the benchmark as I had done before. I'm going to bench it, and if the impact is good enough, I'll update the PR

… slice of bytes

worryg0d · 2026-06-26T14:41:38Z

Alright, I ended up adding reusable buffers to segmentWriter and segmentCodec.

What changed:

segmentCodec.encode now takes [][]byte payload to avoid the use of a temporary intermediate buffer; also, it takes as an argument the destination buffer in which segments should be encoded so that it can be reused afterwards
added scratch buffers to segmentCodec to reduce the amount of memory allocation on the compressed path.

Benchmark results are here: https://docs.google.com/spreadsheets/d/1XKU0Fkw5tPQjhZNtBG86s5n0UumZ1qJUdoViThzzdQ8/edit?usp=sharing

Results are slightly better compared to the previous diff:

p99 SELECT workload around -9% vs -11%
p99 INSERT workload around -9% vs -13%

joao-r-reis · 2026-07-07T12:43:47Z

+
+// decodePayload reads and verifies the payload of a segment from the given reader.
+func (sc *segmentCodec) decodePayload(r io.Reader, header *segmentHeader) ([]byte, error) {
+	payload := make([]byte, header.payloadLength)


we could improve even further by reusing a buffer here too but I'm ok with merging what we already have since it's already a very good improvement

Those reused buffers actually makes the solution more complicated to me imo, so we probably should think about introducing something like buffer pools at some point

joao-r-reis · 2026-07-07T12:49:36Z

+	if c.version >= protoVersion5 {
+		c.logger.Debug("Switching to segments for connection", NewLogFieldStringer("write_timeout", c.session.cfg.WriteTimeout), NewLogFieldStringer("write_coalesce_wait_time", c.session.cfg.WriteCoalesceWaitTime))
+		// Use segments writer which basically batches multiple frames into a single segment before flushing them to the connection.
+		segmentWriter := newSegmentWriter(host.Conn, c.writeTimeout, c.session.cfg.WriteCoalesceWaitTime, c.ctx.Done(), c.compressor)


I believe the proto v4 path checks if writecoalescetime is > 0 before running the coalescer so we should probably guard against this here too

I actually can't see a value of having such feature as it goes against the batching nature of v5 frame formats. However, I'm unsure if it should be tied to the generic WriteCoalesceWaitTime option, probably it's better to introduce a separete SegmentsWriteTimeout

worryg0d force-pushed the cassgo-100 branch 2 times, most recently from 7fb089d to 6d41de3 Compare April 27, 2026 11:46

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

worryg0d marked this pull request as ready for review May 4, 2026 07:34

joao-r-reis reviewed May 19, 2026

View reviewed changes

worryg0d added 8 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

worryg0d added 4 commits June 22, 2026 14:12

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

joao-r-reis reviewed Jul 7, 2026

View reviewed changes

	func (c *connReader) Read(p []byte) (n int, err error) {
	const maxAttempts = 5

	for i := 0; i < maxAttempts; i++ {
	var nn int
	if c.timeout > 0 {
	c.conn.SetReadDeadline(time.Now().Add(c.timeout))
	}

	nn, err = io.ReadFull(c.r, p[n:])
	n += nn
	if err == nil {
	break
	}

	if verr, ok := err.(net.Error); !ok \|\| !verr.Temporary() {
	break
	}
	}

	return
	}

Uh oh!

Conversation

worryg0d commented Apr 24, 2026

Uh oh!

worryg0d commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joao-r-reis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

worryg0d May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

worryg0d commented May 19, 2026

Uh oh!

worryg0d commented May 28, 2026

INSERT (base vs candidate, %)

SELECT (base vs candidate, %)

INSERT (compression) (base vs candidate, %)

SELECT (compression) (base vs candidate, %)

Uh oh!

worryg0d commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Non-compression

INSERT

SELECT

Compression

INSERT

SELECT

Non-compression (proto v4 trunk vs proto v5 trunk vs proto v5 cassgo100)

Compression (proto v4 trunk vs proto v5 trunk vs proto v5 cassgo100)

Uh oh!

worryg0d commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

No compression (LZ4 off)

Latency

INSERT

SELECT

Memory (Go heap)

Heap allocation

With compression (LZ4 on)

Latency

INSERT

SELECT

Memory (Go heap)

Heap allocation

Uh oh!

joao-r-reis commented May 28, 2026

Uh oh!

worryg0d commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

worryg0d commented May 28, 2026

Uh oh!

joao-r-reis commented Jun 18, 2026

worryg0d commented May 4, 2026 •

edited

Loading

worryg0d May 20, 2026 •

edited

Loading

worryg0d commented May 28, 2026 •

edited

Loading

worryg0d commented May 28, 2026 •

edited

Loading

worryg0d commented May 28, 2026 •

edited

Loading