• Add transport/routing.go with ShardInfo
• Add transport.ConnectionConfig for the 1st version we need
  • pub tcp_nodelay: bool,
  • pub connect_timeout: std::time::Duration,
  • pub default_consistency: Consistency,
• Add transport.OpenConnection note that the OpenConnection function family should only do the dialing and Wrap the TCP connection. The protocol handshake shall be implemented as an init method called by WrapConn function.
• We should only support shard aware ports i.e. dialing with source port.

After a closer examination I think that the buffer error reporting is useless.

Write path - it will never happen that we send invalid data.

WriteConsistency
		b.recordError(fmt.Errorf("invalid consistency: %v", v))
WriteEventTypes
			b.recordError(fmt.Errorf("invalid EventType %s", k))
WriteInet
		b.recordError(fmt.Errorf("invalid IP length"))
WriteOpCode
		b.recordError(fmt.Errorf("invalid operation code: %v", v))
WriteStartupOptions
			b.recordError(fmt.Errorf("invalid mandatory Startup option %s: %s", k, s))
			b.recordError(fmt.Errorf("invalid Startup option %s: %s", k, s))
		b.recordError(fmt.Errorf("invalid Startup option"))
WriteValue
		b.recordError(fmt.Errorf("invalid value"))

To save this work for future I suggest adding frame.Debug variable.
It the flag is on we check the condition, if condition is met we log it.

Read path - premature handling of unsupported options.

ReadConsistency
		b.recordError(fmt.Errorf("invalid consistency: %v", v))
ReadErrorCode
		b.recordError(fmt.Errorf("invalid error code: %d", v))
ReadInet
	b.recordError(fmt.Errorf("invalid ip length"))
ReadOpCode
		b.recordError(fmt.Errorf("invalid operation code: %v", o))
ReadOption
			b.recordError(fmt.Errorf("invalid Option ID: %d", id))
ReadSchemaChangeTarget
		b.recordError(fmt.Errorf("invalid SchemaChangeTarget: %s", v))
ReadSchemaChangeType
		b.recordError(fmt.Errorf("invalid SchemaChangeType: %s", t))
ReadStatusChangeType
		b.recordError(fmt.Errorf("invalid StatusChangeType: %s", t))
ReadTopologyChangeType
		b.recordError(fmt.Errorf("invalid TopologyChangeType: %s", t))
ReadValue
		b.recordError(fmt.Errorf("invalid value length"))
ReadWriteType
		b.recordError(fmt.Errorf("invalid write type: %s", w))

If server sent OpCode we do not support the place to handle that is in the main parsing function.
The fact that constant exists does not guarantee that it is supported anyways.
Checking for those errors actually is mixed up with i/o errors wich is unfortunate.
I suggest we do the same as with Write path - use frame.Debug variable.

Read - handling errors that would never happen

read
		b.recordError(fmt.Errorf("buffer read error: %w", err))
		b.recordError(fmt.Errorf("buffer read error: invalid length"))

We can assume that server sends valid data, and buffer always contains all the data needed to parse.
This can be a panic().

To summarize:

• Introduce frame.Debug variable and only do the checks if the flag is true
• Remove Buffer error
• In read add panics if we cannot read enough data

Setup a gh action to run tests.

Usually you should always use parameter name when initializing a struct.

This is OK

	var cases = []struct {
		name     string
		content  []byte
		expected string
	}{
		{"one char", []byte{0x00, 0x01, 0x61}, "a"},
		{"normal word", []byte{0x00, 0x06, 0x67, 0x6f, 0x6c, 0x61, 0x6e, 0x67}, "golang"},
		{"UTF-8 characters", []byte{0x00, 0x0a, 0xcf, 0x80, 0xc5, 0x93, 0xc4, 0x99, 0xc2, 0xa9, 0xc3, 0x9f}, "πœę©ß"},
		{"empty string", []byte{0x00, 0x00}, ""},
	}

This is NOT OK

		{"plain supported",
			[]byte{0x84, 0x0, 0x0, 0x0, 0x06, 0x0, 0x0, 0x0, 0x0},
			Header{
				Version:  CQLv4,
				Flags:    0,
				StreamID: 0,
				Opcode:   OpSupported,
				Length:   0,
			},
		},

and should be replaced with explicit stuct field names.

Example

type BatchTypeFlag = byte

When creating lists and maps use make function to give hists on the collection size.

Move the errors to errors.go in the repo root.
Framer should not try to validate the error code.
The error codes are for driver users to get to know what the error means.

It would be best to have proper errors that can print themselves with descriptions but that we can do later.

The comment is not true, note < vs <=.
The comment is not needed, nil slice is just perfectly fine and valid.

// If read Bytes length is negative returns nil.
func (b *Buffer) ReadBytes() Bytes {
	// Read length of the Bytes.
	n := b.ReadInt()
	if n < 0 {
		return nil
	}
	return b.Read(int(n))
}

Define Session struct in project root dir. Specify control connection, control connection shall discover addresses of all nodes.
Session should then create per host connection pools for all hosts.

• Add connection init called by WrapConn (consult with @Michal-Leszczynski to avoid duplicate work)
• Add query
• Write a test that massively queries a single node

// Length equal to -1 represents null.
// Length equal to -2 represents not set.
func (b *Buffer) ReadValue() Value {
	if n := b.ReadInt(); n < -2 {
		b.RecordError(fmt.Errorf("invalid value length"))
	} else if n > 0 {
		return Value{N: n, Bytes: b.Read(int(n))}
	} else {
		return Value{N: n}
	}
	return Value{}
}

Get rid of the comment and add special Values, ex

var NotSetValue = Value{N: -2}

At the moment the test drops large amounts of queries because Scylla detects the same query repeated over and over.

Jan 28 14:26:56 d0f3d59d1516 scylla:  [shard 0] reader_concurrency_semaphore - (rate limiting dropped 1363 similar messages) Semaphore _read_concurrency_sem with 75/100 count and 1246360/10737418 memory resources: timed out, dumping permit diagnostics:#012permits#011count#011memory#011table/description/state#01274#01174#0111184K#011mykeyspace.users/shard-reader/inactive#012272#0110#01117K#011mykeyspace.users/multishard-mutation-query/active/unused#0121#0111#01116K#011mykeyspace.users/shard-reader/active/used#0127#0110#0110B#011mykeyspace.users/shard-reader/waiting#012#012354#01175#0111217K#011total#012#012Total: 354 permits with 75 count and 1217K memory resources

In stdlib we have testCases and tc let's stick with that.

testCases := []struct {
		r       badReader
		res     string
		err     error
		dbuflen int
	}{
		// Check valid inpu

for _, tc := range testCases {

Add goleak to tests to make sure readers and writer is terminated.

Google's https://pkg.go.dev/github.com/google/go-cmp/cmp is a swiss knife tool to all comparing in tests.

See example in https://pkg.go.dev/github.com/google/go-cmp/cmp#Diff

https://github.com/mmatczuk/scylla-go-driver/runs/4817728040?check_suite_focus=true

Investigate and implement efficient query parameter binding.
Using Bind(...interface{}) is no go due to performance inefficiencies in Go.

This should be frame internal.

Each Buffer method MUST have

if b.err != nil {
    return
}

preambule.

Currently it is missing from some methods, others have

if b.err == nil {
    do sth
    ...
}

Handle the error first then proceed.

As suggested in #24 (comment) replace *bytes.Buffer with *frame.Buffer.

Change frame functions to methods.

Eliminate panics by using the error field.

Actually the error should be of type error not []error and if error is not nil all functions shall return early.

Consider passing huge params by pointer.

https://github.com/mmatczuk/scylla-go-driver/blob/main/golangci.yml#L3

Add ability to bind and scan:

• List
• Map
• Set
• UDT
• Tuple

• Copy gocql murmur package
• Add transport.Token in routing.go
• Assume shard awareness
• Assume 1 connection per shard
• Add NodeConnectionPool to transport/pool.go
• Add PoolRefiller as separate from NodeConnectionPool to transport/pool.go
• Add PoolRefiller support for connection errors i.e. if frame sending failed with i/o error PoolRefiller would know about it and replace the connection

StreamIDAllocator is specified as

type StreamIDAllocator interface {
	Alloc() (frame.StreamID, error)
	Free(id frame.StreamID)
}

Gocql has an implementation based on buckets with atomics.
Rust uses simpler approach (also with buckets) but under a lock.

I'd try:

• Move getting the ID to func (c *connReader) setHandler(streamID frame.StreamID, h responseHandler) it would generate the ID under a lock but we need to take the lock anyways and the scope is only one connection.
• Then we can have simpler implementation like in our Rust driver

Tests are missing.

We decided to reuse the buffer. There are few left over tests that use this.

Example

func (Options) OpCode() frame.OpCode {
	return frame.OpOptions
}

Example

var _ frame.Request = AuthResponse{}

In case we switch to pointer receivers one shall cast nil.

Remove the disabled-checks section and enable all linters.

Rename golangci.yml to golangci.yml and check the results

For me

 michal@lh-2:Scylla/scylla-go-driver (main)$ golangci-lint run ./... | wc -l
WARN [runner] The linter 'golint' is deprecated (since v1.41.0) due to: The repository of the linter has been archived by the owner.  Replaced by revive. 
WARN [runner] The linter 'scopelint' is deprecated (since v1.39.0) due to: The repository of the linter has been deprecated by the owner.  Replaced by exportloopref. 
WARN [runner] The linter 'interfacer' is deprecated (since v1.38.0) due to: The repository of the linter has been archived by the owner.  
WARN [runner] The linter 'maligned' is deprecated (since v1.38.0) due to: The repository of the linter has been archived by the owner.  Replaced by govet 'fieldalignment'. 
688

I guess you want to restore the list of stupid linters, feel free to copy from https://github.com/scylladb/scylla-manager/blob/master/.golangci.yml#L38

UUID = [16]byte

We can then eliminate the length check.

Example map[WriteType]bool should be map[WriteType]struct{}.
The former takes 1 byte the latter 0.

Example

	var x [16]byte
	var v bytes.Buffer
	v.Read(x[:])

Replace b := frame.Buffer{} with var equivalent.

Clients should only use the ReadSth functions.
There are no usages except for buffer itself.

See https://github.com/actions/cache/blob/main/examples.md#go---modules

We have

	l, err := b.buf.Read(p)
	if l != n || err != nil {
		b.recordError(fmt.Errorf("invalid Buffer Read"))
	}

We want

• Invalid len to be reported separately
• Original error to be recorded

Some types have links to docs others do not. Please add the missing bits.

• Use permalink
• Instead of

// https://github.com/apache/cassandra/blob/trunk/doc/native_protocol_v4.spec#L241-L245
type Inet struct

use

// Inet is specified by https://github.com/apache/cassandra/blob/XXX/doc/native_protocol_v4.spec#L241-L245.
type Inet struct {

Relates to https://github.com/gocql/gocql/pull/1584/files