Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
132338: pgwire: reduce allocations when writing CHAR(N) datums r=mgartner a=mgartner

`ResolveBlankPaddedCharBytes` has been replaced with a more efficient
method of padding `CHAR(N)` datums with trailing spaces.

Epic: None

Release note: None


132501: opt: remove outdated  TODO r=mgartner a=mgartner

A TODO that was addressed in #129007 has been removed.

Release note: None

132563: kvserver: skip empty RACv1 dispatches r=kvoli a=pav-kv

This commit skips creating a `RaftMessageRequest` in the fallback RAC admission dispatch code if there are no pending RACv1 dispatches. Previously, it would send an empty `RaftMessageRequest` which would cause an error in the receiver's `handleRaftRequest`, [message drops](https://github.com/cockroachdb/cockroach/blob/9e12f67ff4ad860651c40dcef489a1556d1d11b7/pkg/kv/kvserver/raft_transport.go#L451-L456), and the receiver's message queue to restart after the following warning:

```
unable to accept Raft message from (n0,s0):?: no handler registered for (n0,s0):?
```

Related to #129508

Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Pavel Kalinnikov <[email protected]>
  • Loading branch information
3 people committed Oct 14, 2024
4 parents 7604dbd + 5fb51e4 + 6751516 + 119d5f6 commit 5531ba1
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 18 deletions.
10 changes: 6 additions & 4 deletions pkg/kv/kvserver/raft_transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -885,10 +885,12 @@ func (t *RaftTransport) processQueue(
dispatchPendingFlowTokensTimer.Reset(0)
}

req := newRaftMessageRequest()
maybeAnnotateWithAdmittedRaftLogEntries(req, pendingDispatches)
batch.Requests = append(batch.Requests, *req)
releaseRaftMessageRequest(req)
if len(pendingDispatches) != 0 {
req := newRaftMessageRequest()
maybeAnnotateWithAdmittedRaftLogEntries(req, pendingDispatches)
batch.Requests = append(batch.Requests, *req)
releaseRaftMessageRequest(req)
}

maybeAnnotateWithStoreIDs(batch)
annotateWithClockTimestamp(batch)
Expand Down
1 change: 0 additions & 1 deletion pkg/sql/opt/optbuilder/testdata/scalar
Original file line number Diff line number Diff line change
Expand Up @@ -1664,7 +1664,6 @@ project
└── filters
└── NOT ((t:1 >= t:1) AND (t:1 <= CASE NULL WHEN true THEN c:5 ELSE t:1::BPCHAR END))

# TODO(#108360): implicit casts from STRING to CHAR should use bpchar.
exec-ddl
CREATE TABLE t108360_1 (t TEXT)
----
Expand Down
19 changes: 17 additions & 2 deletions pkg/sql/pgwire/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -515,15 +515,30 @@ func writeBinaryDecimal(b *writeBuffer, v *apd.Decimal) {
}
}

// spaces is used for padding CHAR(N) datums.
var spaces = [16]byte{
' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ',
}

func writeBinaryBytes(b *writeBuffer, v []byte, t *types.T) {
v = tree.ResolveBlankPaddedCharBytes(v, t)
if t.Oid() == oid.T_char && len(v) == 0 {
// Match Postgres and always explicitly include a null byte if we have
// an empty string for the "char" type in the binary format.
v = []byte{0}
}
b.putInt32(int32(len(v)))
pad := 0
if t.Oid() == oid.T_bpchar && len(v) < int(t.Width()) {
// Pad spaces on the right of the byte slice to make it of length
// specified in the type t.
pad = int(t.Width()) - len(v)
}
b.putInt32(int32(len(v) + pad))
b.write(v)
for pad > 0 {
n := min(pad, len(spaces))
b.write(spaces[:n])
pad -= n
}
}

func writeBinaryString(b *writeBuffer, v string, t *types.T) {
Expand Down
11 changes: 11 additions & 0 deletions pkg/sql/pgwire/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,10 @@ func benchmarkWriteColumnarString(b *testing.B, format pgwirebase.FormatCode) {
benchmarkWriteColumnar(b, getBatch(types.String), format)
}

func benchmarkWriteColumnarChar(b *testing.B, format pgwirebase.FormatCode) {
benchmarkWriteColumnar(b, getBatch(types.MakeChar(16)), format)
}

func benchmarkWriteDate(b *testing.B, format pgwirebase.FormatCode) {
d, _, err := tree.ParseDDate(nil, "2010-09-28")
if err != nil {
Expand Down Expand Up @@ -671,6 +675,13 @@ func BenchmarkWriteBinaryColumnarString(b *testing.B) {
benchmarkWriteColumnarString(b, pgwirebase.FormatBinary)
}

func BenchmarkWriteTextColumnarChar(b *testing.B) {
benchmarkWriteColumnarChar(b, pgwirebase.FormatText)
}
func BenchmarkWriteBinaryColumnarChar(b *testing.B) {
benchmarkWriteColumnarChar(b, pgwirebase.FormatBinary)
}

func BenchmarkWriteTextDate(b *testing.B) {
benchmarkWriteDate(b, pgwirebase.FormatText)
}
Expand Down
11 changes: 0 additions & 11 deletions pkg/sql/sem/tree/pgwire_encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,6 @@ func ResolveBlankPaddedChar(s string, t *types.T) string {
return s
}

// ResolveBlankPaddedCharBytes is similar to ResolveBlankPaddedChar but operates
// on a slice of bytes instead of a string.
func ResolveBlankPaddedCharBytes(v []byte, t *types.T) []byte {
if t.Oid() == oid.T_bpchar && len(v) < int(t.Width()) {
// Pad spaces on the right of the byte slice to make it of length
// specified in the type t.
return append(v, bytes.Repeat([]byte(" "), int(t.Width())-len(v))...)
}
return v
}

func (d *DTuple) pgwireFormat(ctx *FmtCtx) {
// When converting a tuple to text in "postgres mode" there is
// special behavior: values are printed in "postgres mode" then the
Expand Down

0 comments on commit 5531ba1

Please sign in to comment.