sql: add a way for sequence caching to be per-node

[jasminejsun]

Previously, the sequence CACHE implementation caches IDs per-session. This can be wasteful since a single increment will allocate extra space within the sequences for each session. Depending on the use cases, a node-level cache would be more effective and minimize the number of wasted sequence values.

Epic: CRDB-20209 Fixes: #89338

Release note (sql change): Added an option for node-level sequence caching. All the sessions on the node can share the same cache, which can be concurrently accessed. The syntax for this option is [ [ PER NODE ] CACHE # ] or serial_normalization setting for sql_sequence_cached_node.

[blathers-crl[bot]]

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

It looks like your PR touches SQL parser code but doesn't add or edit parser tests. Please make sure you add or edit parser tests if you edit the parser.

_{:owl: Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[cockroach-teamcity]

This change is 

[jasminejsun]

pkg/sql/catalog/descpb/structured.go line 303 at r2 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

nit: "it will return 1 if the both fields are 0" so should this be && instead of ||?

oops! i updated the comment. i dont think the cache size can be set to 0 (uninitialized) when using the PER NODE CACHE sequence option, unlike the CACHE option based on this comment: "Prior to #51259, sequence caching was unimplemented and cache sizes were left uninitialized (ie. to have a value of 0).". which is why i used || instead of && (its just a precautionary). the user can't set the value to 0 due to validation.

[jasminejsun]

pkg/sql/logictest/testdata/logic_test/serial line 400 at r2 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

just curious, why did this test get removed in the latest patch?

moved it to pkg/sql/logictest/testdata/logic_test/mixed_version_sequence_per_node_cache!

[jasminejsun]

pkg/sql/alter_sequence.go line 157 at r8 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

i may not follow this: don't we need to look at option.IntVal if the version is active?

I noticed that the NodeCacheSize value was getting assigned on line 87. Example: here I altered the sequence NodeCacheSize from 10->20:

[jasminejsun]

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/create_sequence.go line 76 at r8 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

does this code need to use the value of opt.IntVal if the v24.1 version is active?

same as above comment! call to AssignSequenceOptions on line 90!

[jasminejsun]

bors r+

[craig[bot]]

Build succeeded:

• Bazel Essential CI (Cockroach)

[rickystewart]

Looks like this PR introduced a race build failure in pkg/sql/sessiondatapb/sequence_cache_node.go

pkg/sql/sessiondatapb/sequence_cache_node.go:47:8: sc.mu.RWMutex undefined (type syncutil.RWMutex has no field or method RWMutex)
pkg/sql/sessiondatapb/sequence_cache_node.go:49:8: sc.mu.RWMutex undefined (type syncutil.RWMutex has no field or method RWMutex)
pkg/sql/sessiondatapb/sequence_cache_node.go:52:9: sc.mu.RWMutex undefined (type syncutil.RWMutex has no field or method RWMutex)
pkg/sql/sessiondatapb/sequence_cache_node.go:53:15: sc.mu.RWMutex undefined (type syncutil.RWMutex has no field or method RWMutex)