kv: move splitHealthy to method on grpcTransport #74344
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit moves the `splitHealthy` helper to a method on `grpcTransport`. This allows us to implement the `sort.Interface` interface on the heap-allocated `grpcTransport` and avoid the heap allocation previously incurred by `splitHealthy`. ``` name old time/op new time/op delta KV/Scan/Native/rows=1-10 14.9µs ± 4% 15.0µs ± 4% ~ (p=0.529 n=10+10) KV/Scan/SQL/rows=1-10 95.0µs ± 5% 94.5µs ± 7% ~ (p=0.393 n=10+10) name old alloc/op new alloc/op delta KV/Scan/Native/rows=1-10 6.87kB ± 0% 6.82kB ± 0% -0.71% (p=0.000 n=9+10) KV/Scan/SQL/rows=1-10 20.0kB ± 0% 20.0kB ± 0% -0.30% (p=0.007 n=10+10) name old allocs/op new allocs/op delta KV/Scan/Native/rows=1-10 52.0 ± 0% 51.0 ± 0% -1.92% (p=0.000 n=10+10) KV/Scan/SQL/rows=1-10 245 ± 0% 243 ± 0% -0.49% (p=0.001 n=10+10) ```
|
This change is |
This was referenced Dec 30, 2021
tbg
approved these changes
Jan 3, 2022
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @nvanbenschoten)
|
TFTR! bors r=tbg |
|
Build succeeded: |
craig bot
pushed a commit
that referenced
this pull request
Jan 4, 2022
74211: protectedts: add target field to pts record r=irfansharif,arulajmani a=adityamaru Protected timestamp records are moving away from the notion of protecting spans, and instead will operate on objects. Objects will be defined as: - Cluster - Tenant - Schema objects (database and table) This change deprecates the Spans field on `ptpb.Record`. Additionally, it adds a `oneof` target field that reflects which of the above objects the record corresponds to. This information will be needed by the SQLTranslator/Reconciler in future work to emit SpanConfigurations based on the type of object the job is protecting. Informs: #73727 Release note: None 74313: build: use mode default and valid_archive=False in bazel proto gen r=dt a=dt Prior to this change, there were some known-broken go targets in the tree, defined by `go_proto_library` rules, that were unbuildable for reasons explained below. They existed only to be embedded in other `go_library` targets which could then be built, tested, etc and were not _intended_ to be built or tested on their own, but their presence in the tree still caused problems for workflows that inadvertently tried to build them, such as `build //some/prefix/...` or test-changed-packages. Specifically, some options used in protos to control code generation can result in a generated file that cannot be compiled on its own. For example, several of our proto messages use `gogo.stringer=false` which causes the generated message to not include a String() method, but it still asserts that it implements the Message interface which requires that method. It is expected that the generated file is passed to the go compiler along with a hand-written file that adds the missing method to the message type, so that it them implements the interface. That hand-written file depends on the generated file, e.g. the type is appears in the receiver when defining the String() method, so it too cannot be built on its own. Thus simply wrapping it in a go_library of its own to `embed` in the `go_proto_library` doesn't solve this; now *that* target fails to build. Instead, we can set the `valid_archive=False` flag on the proto compiler used to generate these go_proto_library targets. Doing so causes these targets to no longer yeidld a `GoArchive`, which believes it should be able to be built, and instead only yeild the `GoLibrary` and `GoSource` that can then be embedded in `go_library` to complete it. To use the above, we also switch gazelle's proto mode from `package` to `default`; the latter instructs it to generate these `go_library` wrapper targets that embded the `go_proto_library`. 74345: kv: return RangeIterator by value from constructor r=erikgrinaker,irfansharif a=nvanbenschoten This commit changes `RangeIterator`'s constructor to return the struct by value. The methods on `RangeIterator` continue to use a pointer receiver, but this allows the struct to remain on the stack in some cases and to be embedded in larger structs (e.g. `spanResolverIterator`) in others. As a result, it avoids some heap allocations. ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 92.5µs ± 4% 94.4µs ± 5% ~ (p=0.211 n=9+10) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 20.1kB ± 0% 20.1kB ± 0% ~ (p=0.782 n=10+10) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 244 ± 0% -0.41% (p=0.000 n=10+8) ``` ---- This is part of a collection of assorted micro-optimizations: - #74336 - #74337 - #74338 - #74339 - #74340 - #74341 - #74342 - #74343 - #74344 - #74345 - #74346 - #74347 - #74348 Combined, these changes have the following effect on end-to-end SQL query performance: ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 94.4µs ±10% 92.3µs ±11% -2.20% (p=0.000 n=93+93) KV/Scan/SQL/rows=10-10 102µs ±10% 99µs ±10% -2.16% (p=0.000 n=94+94) KV/Update/SQL/rows=10-10 378µs ±15% 370µs ±11% -2.04% (p=0.003 n=95+91) KV/Insert/SQL/rows=1-10 133µs ±14% 132µs ±12% ~ (p=0.738 n=95+93) KV/Insert/SQL/rows=10-10 197µs ±14% 196µs ±13% ~ (p=0.902 n=95+94) KV/Update/SQL/rows=1-10 186µs ±14% 185µs ±14% ~ (p=0.351 n=94+93) KV/Delete/SQL/rows=1-10 132µs ±13% 132µs ±14% ~ (p=0.473 n=94+94) KV/Delete/SQL/rows=10-10 254µs ±16% 250µs ±16% ~ (p=0.086 n=100+99) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 20.1kB ± 0% 19.1kB ± 1% -4.91% (p=0.000 n=96+96) KV/Scan/SQL/rows=10-10 21.7kB ± 0% 20.7kB ± 1% -4.61% (p=0.000 n=96+97) KV/Delete/SQL/rows=10-10 64.0kB ± 3% 63.7kB ± 3% -0.55% (p=0.000 n=100+100) KV/Update/SQL/rows=1-10 45.8kB ± 1% 45.5kB ± 1% -0.55% (p=0.000 n=97+98) KV/Update/SQL/rows=10-10 105kB ± 1% 105kB ± 1% -0.10% (p=0.008 n=97+98) KV/Delete/SQL/rows=1-10 40.8kB ± 0% 40.7kB ± 0% -0.08% (p=0.001 n=95+96) KV/Insert/SQL/rows=1-10 37.4kB ± 1% 37.4kB ± 0% ~ (p=0.698 n=97+96) KV/Insert/SQL/rows=10-10 76.4kB ± 1% 76.4kB ± 0% ~ (p=0.822 n=99+98) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 217 ± 0% -11.43% (p=0.000 n=95+92) KV/Scan/SQL/rows=10-10 280 ± 0% 252 ± 0% -10.11% (p=0.000 n=75+97) KV/Delete/SQL/rows=10-10 478 ± 0% 459 ± 0% -4.04% (p=0.000 n=94+97) KV/Delete/SQL/rows=1-10 297 ± 1% 287 ± 1% -3.34% (p=0.000 n=97+97) KV/Update/SQL/rows=1-10 459 ± 0% 444 ± 0% -3.27% (p=0.000 n=97+97) KV/Insert/SQL/rows=1-10 291 ± 0% 286 ± 0% -1.72% (p=0.000 n=82+86) KV/Update/SQL/rows=10-10 763 ± 1% 750 ± 1% -1.68% (p=0.000 n=96+98) KV/Insert/SQL/rows=10-10 489 ± 0% 484 ± 0% -1.03% (p=0.000 n=98+98) ``` 74351: sql: don't formatStatementHideConstants when profiling prepared stmt r=rafiss a=nvanbenschoten This commit removes the call to `formatStatementHideConstants` when executing a prepared statement while CPU profiling is enabled. The formatted statement is already available, so there's no need to re-format. Formatting isn't excessively expensive and we do expect CPU profiling to be disruptive, so the cost wasn't a big deal on its own. However, the more important part of the change is that it helps clean up the CPU profiles and make them more representative. 74365: pgwire: retrieve password credentials concurrently with client wait r=rafiss a=knz Release note (performance improvement): CockroachDB now retrieves the password credentials of the SQL client concurrently with waiting for the password response during the authentication exchange. This can yield a small latency reduction in new SQL connections. Co-authored-by: Aditya Maru <[email protected]> Co-authored-by: David Taylor <[email protected]> Co-authored-by: Nathan VanBenschoten <[email protected]> Co-authored-by: Raphael 'kena' Poss <[email protected]>
craig bot
pushed a commit
that referenced
this pull request
Jan 6, 2022
74343: sql: use FastIntMap for QueryState.RangesPerNode r=nvanbenschoten a=nvanbenschoten This commit switches `QueryState.RangesPerNode` from a `map[roachpb.NodeID]int` to a `util.FastIntMap`. This avoids a pair of heap allocation as long as node IDs say below 32 and the number of ranges for each node stays below 14. ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 94.5µs ± 4% 94.1µs ± 6% ~ (p=0.796 n=10+10) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 20.1kB ± 0% 19.9kB ± 0% -0.84% (p=0.000 n=10+9) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 243 ± 0% -0.82% (p=0.000 n=10+9) ``` ---- This is part of a collection of assorted micro-optimizations: - #74336 - #74337 - #74338 - #74339 - #74340 - #74341 - #74342 - #74343 - #74344 - #74345 - #74346 - #74347 - #74348 Combined, these changes have the following effect on end-to-end SQL query performance: ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 94.4µs ±10% 92.3µs ±11% -2.20% (p=0.000 n=93+93) KV/Scan/SQL/rows=10-10 102µs ±10% 99µs ±10% -2.16% (p=0.000 n=94+94) KV/Update/SQL/rows=10-10 378µs ±15% 370µs ±11% -2.04% (p=0.003 n=95+91) KV/Insert/SQL/rows=1-10 133µs ±14% 132µs ±12% ~ (p=0.738 n=95+93) KV/Insert/SQL/rows=10-10 197µs ±14% 196µs ±13% ~ (p=0.902 n=95+94) KV/Update/SQL/rows=1-10 186µs ±14% 185µs ±14% ~ (p=0.351 n=94+93) KV/Delete/SQL/rows=1-10 132µs ±13% 132µs ±14% ~ (p=0.473 n=94+94) KV/Delete/SQL/rows=10-10 254µs ±16% 250µs ±16% ~ (p=0.086 n=100+99) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 20.1kB ± 0% 19.1kB ± 1% -4.91% (p=0.000 n=96+96) KV/Scan/SQL/rows=10-10 21.7kB ± 0% 20.7kB ± 1% -4.61% (p=0.000 n=96+97) KV/Delete/SQL/rows=10-10 64.0kB ± 3% 63.7kB ± 3% -0.55% (p=0.000 n=100+100) KV/Update/SQL/rows=1-10 45.8kB ± 1% 45.5kB ± 1% -0.55% (p=0.000 n=97+98) KV/Update/SQL/rows=10-10 105kB ± 1% 105kB ± 1% -0.10% (p=0.008 n=97+98) KV/Delete/SQL/rows=1-10 40.8kB ± 0% 40.7kB ± 0% -0.08% (p=0.001 n=95+96) KV/Insert/SQL/rows=1-10 37.4kB ± 1% 37.4kB ± 0% ~ (p=0.698 n=97+96) KV/Insert/SQL/rows=10-10 76.4kB ± 1% 76.4kB ± 0% ~ (p=0.822 n=99+98) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 217 ± 0% -11.43% (p=0.000 n=95+92) KV/Scan/SQL/rows=10-10 280 ± 0% 252 ± 0% -10.11% (p=0.000 n=75+97) KV/Delete/SQL/rows=10-10 478 ± 0% 459 ± 0% -4.04% (p=0.000 n=94+97) KV/Delete/SQL/rows=1-10 297 ± 1% 287 ± 1% -3.34% (p=0.000 n=97+97) KV/Update/SQL/rows=1-10 459 ± 0% 444 ± 0% -3.27% (p=0.000 n=97+97) KV/Insert/SQL/rows=1-10 291 ± 0% 286 ± 0% -1.72% (p=0.000 n=82+86) KV/Update/SQL/rows=10-10 763 ± 1% 750 ± 1% -1.68% (p=0.000 n=96+98) KV/Insert/SQL/rows=10-10 489 ± 0% 484 ± 0% -1.03% (p=0.000 n=98+98) ``` 74461: ci: additionally build go-test-teamcity in ci r=rail a=rickystewart It's used in examples-orms. Release note: None 74510: deps: rename/upgrade xdg/scram -> xdg-go/scram r=rafiss,shermanCRL a=knz Prereq for #74301 Release note: None 74529: pgwire: fix an assertion r=rafiss a=knz When a client connects over a unix socket, we don't support TLS. The assertion for this was wrong, it did not properly report the invalid situation to the client. We can't really test this - I was not able to find a single client that gets this wrong. Release note: None Co-authored-by: Nathan VanBenschoten <[email protected]> Co-authored-by: Ricky Stewart <[email protected]> Co-authored-by: Raphael 'kena' Poss <[email protected]>
craig bot
pushed a commit
that referenced
this pull request
Jan 10, 2022
74348: sql: release BatchFlowCoordinator objects to pool r=nvanbenschoten a=nvanbenschoten
This commit adds `BatchFlowCoordinator` objects to their `vectorizedFlowCreator`'s
`releasables` slice, which ensures that their `Release` method is called and that
they are properly recycled.
Before this change, heap profiles and instrumentation both indicated that these
objects were not being recycled as intended. For example, heap profiles looked like:
```
Type: alloc_objects
Time: Dec 30, 2021 at 2:10pm (EST)
Active filters:
focus=Pool\).Get
Showing nodes accounting for 92189, 0.57% of 16223048 total
----------------------------------------------------------+-------------
flat flat% sum% cum cum% calls calls% + context
----------------------------------------------------------+-------------
71505 79.69% | github.com/cockroachdb/cockroach/pkg/sql/colflow.NewBatchFlowCoordinator /Users/nathan/Go/src/github.com/cockroachdb/cockroach/pkg/sql/colflow/flow_coordinator.go:218
10923 12.17% | github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency.newLockTableGuardImpl /Users/nathan/Go/src/github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/lock_table.go:452
2048 2.28% | github.com/cockroachdb/cockroach/pkg/sql/catalog/nstree.(*Map).maybeInitialize /Users/nathan/Go/src/github.com/cockroachdb/cockroach/pkg/sql/catalog/nstree/map.go:112
1170 1.30% | github.com/cockroachdb/cockroach/pkg/sql/colexec.newMaterializerInternal /Users/nathan/Go/src/github.com/cockroachdb/cockroach/pkg/sql/colexec/materializer.go:179
1030 1.15% | github.com/cockroachdb/pebble.(*Iterator).Clone /Users/nathan/Go/src/github.com/cockroachdb/cockroach/vendor/github.com/cockroachdb/pebble/iterator.go:1228
585 0.65% | github.com/cockroachdb/cockroach/pkg/sql.MakeDistSQLReceiver /Users/nathan/Go/src/github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:841
585 0.65% | github.com/cockroachdb/cockroach/pkg/sql/colfetcher.NewColBatchScan /Users/nathan/Go/src/github.com/cockroachdb/cockroach/pkg/sql/colfetcher/colbatch_scan.go:223
585 0.65% | github.com/cockroachdb/cockroach/pkg/sql/span.MakeBuilder /Users/nathan/Go/src/github.com/cockroachdb/cockroach/pkg/sql/span/span_builder.go:61
585 0.65% | github.com/cockroachdb/cockroach/pkg/storage.mvccGet /Users/nathan/Go/src/github.com/cockroachdb/cockroach/pkg/storage/mvcc.go:859
585 0.65% | github.com/cockroachdb/cockroach/pkg/storage.mvccScanToBytes /Users/nathan/Go/src/github.com/cockroachdb/cockroach/pkg/storage/mvcc.go:2357
128 0.14% | github.com/cockroachdb/pebble.(*DB).newIterInternal /Users/nathan/Go/src/github.com/cockroachdb/cockroach/vendor/github.com/cockroachdb/pebble/db.go:765
0 0% 0% 89729 0.55% | sync.(*Pool).Get /Users/nathan/Go/go/src/sync/pool.go:148
```
Notice that `sync.(*Pool).Get` is responsible for *0.55%* of heap allocations and
**79.69%** of these are from `colflow.NewBatchFlowCoordinator`.
After this change, that source of allocations goes away and we see the following
impact on micro-benchmarks:
```
name old time/op new time/op delta
KV/Scan/SQL/rows=1-10 95.1µs ± 7% 95.9µs ± 5% ~ (p=0.579 n=10+10)
KV/Scan/SQL/rows=10-10 100µs ± 3% 103µs ±12% ~ (p=0.829 n=8+10)
name old alloc/op new alloc/op delta
KV/Scan/SQL/rows=10-10 21.7kB ± 0% 21.5kB ± 0% -0.76% (p=0.000 n=10+10)
KV/Scan/SQL/rows=1-10 20.1kB ± 0% 19.9kB ± 0% -0.70% (p=0.000 n=10+9)
name old allocs/op new allocs/op delta
KV/Scan/SQL/rows=1-10 245 ± 0% 244 ± 0% -0.41% (p=0.000 n=10+10)
KV/Scan/SQL/rows=10-10 280 ± 0% 279 ± 0% -0.36% (p=0.001 n=8+9)
```
----
This is part of a collection of assorted micro-optimizations:
- #74336
- #74337
- #74338
- #74339
- #74340
- #74341
- #74342
- #74343
- #74344
- #74345
- #74346
- #74347
- #74348
Combined, these changes have the following effect on end-to-end SQL query performance:
```
name old time/op new time/op delta
KV/Scan/SQL/rows=1-10 94.4µs ±10% 92.3µs ±11% -2.20% (p=0.000 n=93+93)
KV/Scan/SQL/rows=10-10 102µs ±10% 99µs ±10% -2.16% (p=0.000 n=94+94)
KV/Update/SQL/rows=10-10 378µs ±15% 370µs ±11% -2.04% (p=0.003 n=95+91)
KV/Insert/SQL/rows=1-10 133µs ±14% 132µs ±12% ~ (p=0.738 n=95+93)
KV/Insert/SQL/rows=10-10 197µs ±14% 196µs ±13% ~ (p=0.902 n=95+94)
KV/Update/SQL/rows=1-10 186µs ±14% 185µs ±14% ~ (p=0.351 n=94+93)
KV/Delete/SQL/rows=1-10 132µs ±13% 132µs ±14% ~ (p=0.473 n=94+94)
KV/Delete/SQL/rows=10-10 254µs ±16% 250µs ±16% ~ (p=0.086 n=100+99)
name old alloc/op new alloc/op delta
KV/Scan/SQL/rows=1-10 20.1kB ± 0% 19.1kB ± 1% -4.91% (p=0.000 n=96+96)
KV/Scan/SQL/rows=10-10 21.7kB ± 0% 20.7kB ± 1% -4.61% (p=0.000 n=96+97)
KV/Delete/SQL/rows=10-10 64.0kB ± 3% 63.7kB ± 3% -0.55% (p=0.000 n=100+100)
KV/Update/SQL/rows=1-10 45.8kB ± 1% 45.5kB ± 1% -0.55% (p=0.000 n=97+98)
KV/Update/SQL/rows=10-10 105kB ± 1% 105kB ± 1% -0.10% (p=0.008 n=97+98)
KV/Delete/SQL/rows=1-10 40.8kB ± 0% 40.7kB ± 0% -0.08% (p=0.001 n=95+96)
KV/Insert/SQL/rows=1-10 37.4kB ± 1% 37.4kB ± 0% ~ (p=0.698 n=97+96)
KV/Insert/SQL/rows=10-10 76.4kB ± 1% 76.4kB ± 0% ~ (p=0.822 n=99+98)
name old allocs/op new allocs/op delta
KV/Scan/SQL/rows=1-10 245 ± 0% 217 ± 0% -11.43% (p=0.000 n=95+92)
KV/Scan/SQL/rows=10-10 280 ± 0% 252 ± 0% -10.11% (p=0.000 n=75+97)
KV/Delete/SQL/rows=10-10 478 ± 0% 459 ± 0% -4.04% (p=0.000 n=94+97)
KV/Delete/SQL/rows=1-10 297 ± 1% 287 ± 1% -3.34% (p=0.000 n=97+97)
KV/Update/SQL/rows=1-10 459 ± 0% 444 ± 0% -3.27% (p=0.000 n=97+97)
KV/Insert/SQL/rows=1-10 291 ± 0% 286 ± 0% -1.72% (p=0.000 n=82+86)
KV/Update/SQL/rows=10-10 763 ± 1% 750 ± 1% -1.68% (p=0.000 n=96+98)
KV/Insert/SQL/rows=10-10 489 ± 0% 484 ± 0% -1.03% (p=0.000 n=98+98)
```
Co-authored-by: Nathan VanBenschoten <[email protected]>
craig bot
pushed a commit
that referenced
this pull request
Jan 10, 2022
74341: sql/catalog: restore fast-path in FullIndexColumnIDs r=ajwerner a=nvanbenschoten This commit restores a [fast-path](c9e116e#diff-19625608f4a6e23e6fe0818f3a621e716615765cb338d18fe34b43f0a535f06dL140) in `FullIndexColumnIDs` which was lost in c9e116e. The fast-path avoided the allocation of a `ColumnID` slice and a `IndexDescriptor_Direction` slice in `FullIndexColumnIDs` when given a unique index. In such cases, these slices are already stored on the `IndexDescriptor`. ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 94.9µs ±10% 94.9µs ± 8% ~ (p=0.739 n=10+10) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 20.1kB ± 0% 20.1kB ± 1% ~ (p=0.424 n=10+10) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 241 ± 0% -1.63% (p=0.000 n=10+8) ``` ---- This is part of a collection of assorted micro-optimizations: - #74336 - #74337 - #74338 - #74339 - #74340 - #74341 - #74342 - #74343 - #74344 - #74345 - #74346 - #74347 - #74348 Combined, these changes have the following effect on end-to-end SQL query performance: ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 94.4µs ±10% 92.3µs ±11% -2.20% (p=0.000 n=93+93) KV/Scan/SQL/rows=10-10 102µs ±10% 99µs ±10% -2.16% (p=0.000 n=94+94) KV/Update/SQL/rows=10-10 378µs ±15% 370µs ±11% -2.04% (p=0.003 n=95+91) KV/Insert/SQL/rows=1-10 133µs ±14% 132µs ±12% ~ (p=0.738 n=95+93) KV/Insert/SQL/rows=10-10 197µs ±14% 196µs ±13% ~ (p=0.902 n=95+94) KV/Update/SQL/rows=1-10 186µs ±14% 185µs ±14% ~ (p=0.351 n=94+93) KV/Delete/SQL/rows=1-10 132µs ±13% 132µs ±14% ~ (p=0.473 n=94+94) KV/Delete/SQL/rows=10-10 254µs ±16% 250µs ±16% ~ (p=0.086 n=100+99) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 20.1kB ± 0% 19.1kB ± 1% -4.91% (p=0.000 n=96+96) KV/Scan/SQL/rows=10-10 21.7kB ± 0% 20.7kB ± 1% -4.61% (p=0.000 n=96+97) KV/Delete/SQL/rows=10-10 64.0kB ± 3% 63.7kB ± 3% -0.55% (p=0.000 n=100+100) KV/Update/SQL/rows=1-10 45.8kB ± 1% 45.5kB ± 1% -0.55% (p=0.000 n=97+98) KV/Update/SQL/rows=10-10 105kB ± 1% 105kB ± 1% -0.10% (p=0.008 n=97+98) KV/Delete/SQL/rows=1-10 40.8kB ± 0% 40.7kB ± 0% -0.08% (p=0.001 n=95+96) KV/Insert/SQL/rows=1-10 37.4kB ± 1% 37.4kB ± 0% ~ (p=0.698 n=97+96) KV/Insert/SQL/rows=10-10 76.4kB ± 1% 76.4kB ± 0% ~ (p=0.822 n=99+98) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 217 ± 0% -11.43% (p=0.000 n=95+92) KV/Scan/SQL/rows=10-10 280 ± 0% 252 ± 0% -10.11% (p=0.000 n=75+97) KV/Delete/SQL/rows=10-10 478 ± 0% 459 ± 0% -4.04% (p=0.000 n=94+97) KV/Delete/SQL/rows=1-10 297 ± 1% 287 ± 1% -3.34% (p=0.000 n=97+97) KV/Update/SQL/rows=1-10 459 ± 0% 444 ± 0% -3.27% (p=0.000 n=97+97) KV/Insert/SQL/rows=1-10 291 ± 0% 286 ± 0% -1.72% (p=0.000 n=82+86) KV/Update/SQL/rows=10-10 763 ± 1% 750 ± 1% -1.68% (p=0.000 n=96+98) KV/Insert/SQL/rows=10-10 489 ± 0% 484 ± 0% -1.03% (p=0.000 n=98+98) ``` 74355: kv: protect Replica's lastToReplica and lastFromReplica fields with raftMu r=nvanbenschoten a=nvanbenschoten This commit moves the Replica's lastToReplica and lastFromReplica from under the `Replica.mu` mutex to the `Replica.raftMu` mutex. These are strictly Raft-specific pieces of state, so we don't need fine-grained locking around them. As a reward, we don't need to grab the `Replica.mu` exclusively (or at all) when setting the fields in `Store.withReplicaForRequest`. The locking in `setLastReplicaDescriptors` showed up in a mutex profile under a write-heavy workload. It was responsible for **3.44%** of mutex wait time. Grabbing the mutex was probably also slowing down request processing, as the exclusive lock acquisition had to wait for read locks to be dropped. <img width="1584" alt="Screen Shot 2021-12-30 at 9 45 08 PM" src="https://user-images.githubusercontent.com/5438456/147800455-8da74dfd-5fd3-4831-818c-7e3c65763435.png"> 74592: coldata: operate on Nulls value, not reference r=yuzefovich a=nvanbenschoten This commit changes `col.Vec.SetNulls` to accept a `Nulls` struct by value instead of by pointer. This lets us avoid a heap allocation on each call to `Nulls.Or`. We saw this in the "after" heap profiles in #74590, which looked like: <img width="1749" alt="Screen Shot 2022-01-07 at 7 17 32 PM" src="https://user-images.githubusercontent.com/5438456/148624263-777a6d93-4df7-40da-84a3-18d5e47ab633.png"> ``` File: cockroach Type: alloc_objects Time: Jan 8, 2022 at 12:17am (UTC) Showing nodes accounting for 5943494, 100% of 5943494 total ----------------------------------------------------------+------------- flat flat% sum% cum cum% calls calls% + context ----------------------------------------------------------+------------- 843873 48.47% | github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecproj.projPlusDecimalDecimalOp.Next.func1 /go/src/github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecproj/proj_non_const_ops.eg.go:3938 823389 47.29% | github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecproj.projPlusInt64Int64Op.Next.func1 /go/src/github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecproj/proj_non_const_ops.eg.go:5732 73736 4.24% | github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecproj.projPlusInt64DecimalOp.Next.func1 /go/src/github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecproj/proj_non_const_ops.eg.go:5870 1740998 29.29% 29.29% 1740998 29.29% | github.com/cockroachdb/cockroach/pkg/col/coldata.(*Nulls).Or /go/src/github.com/cockroachdb/cockroach/pkg/col/coldata/nulls.go:350 ----------------------------------------------------------+------------- 819219 49.50% | github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecproj.projPlusInt64Int64Op.Next.func1 /go/src/github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecproj/proj_non_const_ops.eg.go:5732 704530 42.57% | github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecproj.projPlusDecimalDecimalOp.Next.func1 /go/src/github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecproj/proj_non_const_ops.eg.go:3938 131076 7.92% | github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecproj.projPlusInt64DecimalOp.Next.func1 /go/src/github.com/cockroachdb/cockroach/pkg/sql/colexec/colexecproj/proj_non_const_ops.eg.go:5870 1654825 27.84% 57.14% 1654825 27.84% | github.com/cockroachdb/cockroach/pkg/col/coldata.(*Nulls).Or /go/src/github.com/cockroachdb/cockroach/pkg/col/coldata/nulls.go:348 ----------------------------------------------------------+------------- ``` This PR eliminates one of these two heap allocations. Co-authored-by: Nathan VanBenschoten <[email protected]>
craig bot
pushed a commit
that referenced
this pull request
Jul 11, 2022
74342: storage: short-circuit in recordIteratorStats if not recording r=nvanbenschoten a=nvanbenschoten This commit adds a short-circuit fast-path in recordIteratorStats that avoids a call to `tracing.Span.RecordStructured` when the tracing span is not recording. This avoids a heap allocation in the common case where tracing is not enabled. ``` name old time/op new time/op delta KV/Scan/Native/rows=1-10 18.8µs ± 2% 18.5µs ± 3% -1.14% (p=0.003 n=20+19) KV/Scan/SQL/rows=1-10 95.4µs ± 3% 95.7µs ± 5% ~ (p=0.602 n=20+20) name old alloc/op new alloc/op delta KV/Scan/Native/rows=1-10 7.42kB ± 1% 7.35kB ± 1% -1.01% (p=0.000 n=20+20) KV/Scan/SQL/rows=1-10 23.5kB ± 0% 23.4kB ± 0% -0.31% (p=0.000 n=19+20) name old allocs/op new allocs/op delta KV/Scan/Native/rows=1-10 58.0 ± 0% 57.0 ± 0% -1.72% (p=0.000 n=19+20) KV/Scan/SQL/rows=1-10 279 ± 0% 278 ± 0% -0.36% (p=0.000 n=18+19) ``` ---- This is part of a collection of assorted micro-optimizations: - #74336 - #74337 - #74338 - #74339 - #74340 - #74341 - #74342 - #74343 - #74344 - #74345 - #74346 - #74347 - #74348 Combined, these changes have the following effect on end-to-end SQL query performance: ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 94.4µs ±10% 92.3µs ±11% -2.20% (p=0.000 n=93+93) KV/Scan/SQL/rows=10-10 102µs ±10% 99µs ±10% -2.16% (p=0.000 n=94+94) KV/Update/SQL/rows=10-10 378µs ±15% 370µs ±11% -2.04% (p=0.003 n=95+91) KV/Insert/SQL/rows=1-10 133µs ±14% 132µs ±12% ~ (p=0.738 n=95+93) KV/Insert/SQL/rows=10-10 197µs ±14% 196µs ±13% ~ (p=0.902 n=95+94) KV/Update/SQL/rows=1-10 186µs ±14% 185µs ±14% ~ (p=0.351 n=94+93) KV/Delete/SQL/rows=1-10 132µs ±13% 132µs ±14% ~ (p=0.473 n=94+94) KV/Delete/SQL/rows=10-10 254µs ±16% 250µs ±16% ~ (p=0.086 n=100+99) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 20.1kB ± 0% 19.1kB ± 1% -4.91% (p=0.000 n=96+96) KV/Scan/SQL/rows=10-10 21.7kB ± 0% 20.7kB ± 1% -4.61% (p=0.000 n=96+97) KV/Delete/SQL/rows=10-10 64.0kB ± 3% 63.7kB ± 3% -0.55% (p=0.000 n=100+100) KV/Update/SQL/rows=1-10 45.8kB ± 1% 45.5kB ± 1% -0.55% (p=0.000 n=97+98) KV/Update/SQL/rows=10-10 105kB ± 1% 105kB ± 1% -0.10% (p=0.008 n=97+98) KV/Delete/SQL/rows=1-10 40.8kB ± 0% 40.7kB ± 0% -0.08% (p=0.001 n=95+96) KV/Insert/SQL/rows=1-10 37.4kB ± 1% 37.4kB ± 0% ~ (p=0.698 n=97+96) KV/Insert/SQL/rows=10-10 76.4kB ± 1% 76.4kB ± 0% ~ (p=0.822 n=99+98) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 217 ± 0% -11.43% (p=0.000 n=95+92) KV/Scan/SQL/rows=10-10 280 ± 0% 252 ± 0% -10.11% (p=0.000 n=75+97) KV/Delete/SQL/rows=10-10 478 ± 0% 459 ± 0% -4.04% (p=0.000 n=94+97) KV/Delete/SQL/rows=1-10 297 ± 1% 287 ± 1% -3.34% (p=0.000 n=97+97) KV/Update/SQL/rows=1-10 459 ± 0% 444 ± 0% -3.27% (p=0.000 n=97+97) KV/Insert/SQL/rows=1-10 291 ± 0% 286 ± 0% -1.72% (p=0.000 n=82+86) KV/Update/SQL/rows=10-10 763 ± 1% 750 ± 1% -1.68% (p=0.000 n=96+98) KV/Insert/SQL/rows=10-10 489 ± 0% 484 ± 0% -1.03% (p=0.000 n=98+98) ``` 83796: sql/stats: convert between histograms and quantile functions r=mgartner,rytaft,yuzefovich a=michae2 To predict histograms in statistics forecasts, we will use linear regression over quantile functions. (Quantile functions are another representation of histogram data, in a form more amenable to statistical manipulation.) This commit defines quantile functions and adds methods to convert between histograms and quantile functions. This code was originally part of #77070 but has been pulled out to simplify that PR. A few changes have been made: - Common code has been factored into closures. - More checks have been added for positive values. - In `makeQuantile` we now trim leading empty buckets as well as trailing empty buckets. - The logic in `quantile.toHistogram` to steal from `NumRange` if `NumEq` is zero now checks that `NumRange` will still be >= 1. - More tests have been added. Assists: #79872 Release note: None 83827: asim: update range size split threshold, more keys r=kvoli a=kvoli This patch updates the range size split threshold to 512mb by default. Additionally it adds support for initializing a testing replica distribution, where the ranges contain an equal span of the keyspace. Release note: None 83891: sql: sql parser for `CREATE FUNCTION` statement r=chengxiong-ruan a=chengxiong-ruan handles #83218 This commit added parser support for `CREATE FUNCTION` sql statement. Scanner was extended so that it can recognize the `BEGIN ATOMIC` context so that it doesnot return early when it sees `;` charater which normally indicates the end of a statement. Release note (sql change): `CREATE FUNCTION` statement now can be parsed by crdb, but an unimplemented error would be thrown since the statement processing is not done yet. 83913: pkg/ui: Don't force tracez tags to uppercase. r=benbardin a=benbardin Also, deprecate uses of db-console/.../Badge in favor of the identical version in cluster-ui. <img width="1476" alt="Screen Shot 2022-07-06 at 1 21 50 PM" src="https://user-images.githubusercontent.com/261508/177608120-e7360b9e-d2dd-4e50-8bfe-de97a0d61adf.png"> Release note: None 83929: pgwire: use better error for invalid Describe message r=ZhouXing19 a=rafiss fixes #82785 Release note: None 84165: gc: fix NumKeysAffected counting more than collected r=erikgrinaker a=aliher1911 Previously if key is not collected after a GC batch is sent out it would still be included as affected in GC stats. Those stats are mostly used for logging and tests, the unfortunate effect is that randomized test could fail. This commit fixes the bug. Release note: None Fixes #84164 Co-authored-by: Nathan VanBenschoten <[email protected]> Co-authored-by: Michael Erickson <[email protected]> Co-authored-by: Austen McClernon <[email protected]> Co-authored-by: Chengxiong Ruan <[email protected]> Co-authored-by: Ben Bardin <[email protected]> Co-authored-by: Rafi Shamim <[email protected]> Co-authored-by: Oleg Afanasyev <[email protected]>
craig bot
pushed a commit
that referenced
this pull request
Aug 10, 2022
74337: kv: pool rangeCacheKey objects r=arulajmani a=nvanbenschoten This commit introduces a `sync.Pool` for `rangeCacheKey` objects. This is used to avoid heap allocations when querying and updating the `RangeCache`. ``` name old time/op new time/op delta KV/Scan/Native/rows=1-10 14.8µs ± 2% 14.9µs ± 4% ~ (p=0.356 n=9+10) KV/Scan/SQL/rows=1-10 92.1µs ± 4% 94.3µs ± 5% ~ (p=0.113 n=9+10) name old alloc/op new alloc/op delta KV/Scan/Native/rows=1-10 6.87kB ± 0% 6.85kB ± 0% -0.35% (p=0.000 n=10+10) KV/Scan/SQL/rows=1-10 20.0kB ± 0% 20.0kB ± 0% -0.25% (p=0.012 n=10+10) name old allocs/op new allocs/op delta KV/Scan/Native/rows=1-10 52.0 ± 0% 51.0 ± 0% -1.92% (p=0.000 n=10+10) KV/Scan/SQL/rows=1-10 244 ± 0% 242 ± 0% -0.78% (p=0.000 n=10+10) ``` ---- This is part of a collection of assorted micro-optimizations: - #74336 - #74337 - #74338 - #74339 - #74340 - #74341 - #74342 - #74343 - #74344 - #74345 - #74346 - #74347 - #74348 Combined, these changes have the following effect on end-to-end SQL query performance: ``` name old time/op new time/op delta KV/Scan/SQL/rows=1-10 94.4µs ±10% 92.3µs ±11% -2.20% (p=0.000 n=93+93) KV/Scan/SQL/rows=10-10 102µs ±10% 99µs ±10% -2.16% (p=0.000 n=94+94) KV/Update/SQL/rows=10-10 378µs ±15% 370µs ±11% -2.04% (p=0.003 n=95+91) KV/Insert/SQL/rows=1-10 133µs ±14% 132µs ±12% ~ (p=0.738 n=95+93) KV/Insert/SQL/rows=10-10 197µs ±14% 196µs ±13% ~ (p=0.902 n=95+94) KV/Update/SQL/rows=1-10 186µs ±14% 185µs ±14% ~ (p=0.351 n=94+93) KV/Delete/SQL/rows=1-10 132µs ±13% 132µs ±14% ~ (p=0.473 n=94+94) KV/Delete/SQL/rows=10-10 254µs ±16% 250µs ±16% ~ (p=0.086 n=100+99) name old alloc/op new alloc/op delta KV/Scan/SQL/rows=1-10 20.1kB ± 0% 19.1kB ± 1% -4.91% (p=0.000 n=96+96) KV/Scan/SQL/rows=10-10 21.7kB ± 0% 20.7kB ± 1% -4.61% (p=0.000 n=96+97) KV/Delete/SQL/rows=10-10 64.0kB ± 3% 63.7kB ± 3% -0.55% (p=0.000 n=100+100) KV/Update/SQL/rows=1-10 45.8kB ± 1% 45.5kB ± 1% -0.55% (p=0.000 n=97+98) KV/Update/SQL/rows=10-10 105kB ± 1% 105kB ± 1% -0.10% (p=0.008 n=97+98) KV/Delete/SQL/rows=1-10 40.8kB ± 0% 40.7kB ± 0% -0.08% (p=0.001 n=95+96) KV/Insert/SQL/rows=1-10 37.4kB ± 1% 37.4kB ± 0% ~ (p=0.698 n=97+96) KV/Insert/SQL/rows=10-10 76.4kB ± 1% 76.4kB ± 0% ~ (p=0.822 n=99+98) name old allocs/op new allocs/op delta KV/Scan/SQL/rows=1-10 245 ± 0% 217 ± 0% -11.43% (p=0.000 n=95+92) KV/Scan/SQL/rows=10-10 280 ± 0% 252 ± 0% -10.11% (p=0.000 n=75+97) KV/Delete/SQL/rows=10-10 478 ± 0% 459 ± 0% -4.04% (p=0.000 n=94+97) KV/Delete/SQL/rows=1-10 297 ± 1% 287 ± 1% -3.34% (p=0.000 n=97+97) KV/Update/SQL/rows=1-10 459 ± 0% 444 ± 0% -3.27% (p=0.000 n=97+97) KV/Insert/SQL/rows=1-10 291 ± 0% 286 ± 0% -1.72% (p=0.000 n=82+86) KV/Update/SQL/rows=10-10 763 ± 1% 750 ± 1% -1.68% (p=0.000 n=96+98) KV/Insert/SQL/rows=10-10 489 ± 0% 484 ± 0% -1.03% (p=0.000 n=98+98) ``` 85730: kvserver: also block LEARNER snaps to paused followers r=erikgrinaker a=tbg We checked whether the snapshot recipient was paused only in the raft log queue path. By pushing the check down into `sendSnapshot`, it is now hit by any snapshot attempt, which includes the replicate queue and store rebalancer. For best results, both of these should avoid moving replicas to paused followers in the first place, which they already do, at least partially, so this change shouldn't have much of an impact in practice. Fixes #85479. Release note: None 85732: kvserver: only pause followers when holding active lease r=erikgrinaker a=tbg If the raft leader is not the leaseholder (which includes the case in which we just transferred the lease away), leave all followers unpaused. Otherwise, the leaseholder won't learn that the entries it submitted were committed which effectively causes range unavailability. Fixes #84884. Release note: None 85756: builtins: add strptime/strftime builtins without experimental prefix r=rafiss a=rafiss refs #52139 These are just an alias for the existing implementation. Release note (sql change): The strptime and strftime builtin functions were added as aliases for experimental_strptime and experimental_strftime. Co-authored-by: Nathan VanBenschoten <[email protected]> Co-authored-by: Tobias Grieger <[email protected]> Co-authored-by: Rafi Shamim <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit moves the
splitHealthyhelper to a method ongrpcTransport. This allows us to implement thesort.Interfaceinterface on the heap-allocated
grpcTransportand avoid the heapallocation previously incurred by
splitHealthy.This is part of a collection of assorted micro-optimizations:
Combined, these changes have the following effect on end-to-end SQL query performance: