Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature flag for txn id cache #76329

Closed
Tracked by #74485
maryliag opened this issue Feb 9, 2022 · 0 comments · Fixed by #76523
Closed
Tracked by #74485

feature flag for txn id cache #76329

maryliag opened this issue Feb 9, 2022 · 0 comments · Fixed by #76523
Assignees
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@maryliag
Copy link
Contributor

maryliag commented Feb 9, 2022

Create a feature flag for TXN ID cache added on #74115, so it can be shut off if necessary.

@maryliag maryliag added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-observability labels Feb 9, 2022
Azhng added a commit to Azhng/cockroach that referenced this issue Feb 14, 2022
Resolves cockroachdb#76329

Release note (sql change): new boolean cluster setting
`sql.contention.txn_id_cache.enabled` can be used to turn on/off
transaction ID cache. The default value of this new cluster setting
is `true`.
Azhng added a commit to Azhng/cockroach that referenced this issue Feb 14, 2022
Resolves cockroachdb#76329

Release note (sql change): new boolean cluster setting
`sql.contention.txn_id_cache.enabled` can be used to turn on/off
transaction ID cache. The default value of this new cluster setting
is `true`.
Azhng added a commit to Azhng/cockroach that referenced this issue Feb 14, 2022
Resolves cockroachdb#76329

Release note (sql change): when `sql.contention.txn_id_cache.max_size`
is set to 0, it would effectively turn off transaction ID cache.
Azhng added a commit to Azhng/cockroach that referenced this issue Feb 14, 2022
Resolves cockroachdb#76329

Release note (sql change): when `sql.contention.txn_id_cache.max_size`
is set to 0, it would effectively turn off transaction ID cache.
Azhng added a commit to Azhng/cockroach that referenced this issue Feb 15, 2022
Resolves cockroachdb#76329

Release note (sql change): when `sql.contention.txn_id_cache.max_size`
is set to 0, it would effectively turn off transaction ID cache.
Azhng added a commit to Azhng/cockroach that referenced this issue Feb 16, 2022
Resolves cockroachdb#76329

Release note (sql change): when `sql.contention.txn_id_cache.max_size`
is set to 0, it would effectively turn off transaction ID cache.
craig bot pushed a commit that referenced this issue Feb 16, 2022
76279: sql,kvserver: stop gossiping the system config r=ajwerner a=ajwerner

The first commit removes the system config gossip trigger and the client code to set it,
after a version gate has been passed. The primary major change is that in 22.1 binaries, we
no longer rely on the gossip data at any point, instead we adopt the systemconfigwatcher which
is backed by a rangefeed. Part of this change is to adopt that separate data source in the
`GossipSubscription` implementation of the `Connector` API. 

Subsequently, a number of tests needed updating. 

Remove sql.catalog.unsafe_skip_system_config_trigger.enabled
This cluster setting is no longer useful.

Fixes #54477.
Fixes #70560.

Release note (sql change): The limitation that schema change statements in a transaction could not follow
DML statements has been lifted. 

76523: sql: add option to enable/disable txn id cache  r=Azhng a=Azhng

Reviewer note: only last commit is relevant

---

Resolves #76329

Release note (sql change): when `sql.contention.txn_id_cache.max_size`
is set to 0, it would effectively turn off transaction ID cache.

76613: ccl/sqlproxyccl: cleanup PG interceptor APIs r=JeffSwenson a=jaylim-crl

Informs #76000.

#### ccl/sqlproxyccl: update ForwardMsg to take in an io.Writer

Previously, interceptors connect a source to a destination. This can be
awkward during connection migration because we will now need to update the
destination for an existing interceptor. Adding an UpdateWriter (or similar)
does not seem right.

This PR changes how interceptors work. Instead of connecting one end to
another, interceptors are now one sided only. When attempting a write through
ForwardMsg, callers will need to pass in a destination of type io.Writer,
resulting in a much cleaner API when it comes to connection migration. We
will also remove WriteMsg in this commit. This makes interceptors purely
readers wrapping net.Conn objects.

Since interceptors no longer have destinations, keeping the closed field is
not very useful, so this commit removes that as well. A warning has been
added to ensure that callers do not reuse interceptors or destinations whenever
a ReadMsg or ForwardMsg call returns an error.

#### ccl/sqlproxyccl: replace errPanicWriter with a writer that returns an error

Previously, we had an errPanicWriter struct that panics whenever a Write
call was made. This is used because Receive on the pgproto3 backend and
frontend instances must not call Write. However, panics are not ideal, so
this commit replaces that with a regular writer that just returns an error
when Write is called.

#### ccl/sqlproxyccl: use a default buffer size of 8K within the interceptors 

Previously, we returned an error whenever callers attempt to create
interceptors with a small buffer size. This case is very uncommon, and the API
can be awkward since we now need to handle the error case. To address that,
this commit updates the interceptor's behavior such that we default to an 8K
buffer whenever a buffer size smaller than 5 bytes is used. Since sqlproxy is
the only user, this seems to be a reasonable tradeoff.

At the same time, we also make the specialized interceptors to default to
a buffer size of 8K bytes.

Release note: None

Co-authored-by: Andrew Werner <[email protected]>
Co-authored-by: Azhng <[email protected]>
Co-authored-by: Jay <[email protected]>
@craig craig bot closed this as completed in cb35150 Feb 16, 2022
RajivTS pushed a commit to RajivTS/cockroach that referenced this issue Mar 6, 2022
Resolves cockroachdb#76329

Release note (sql change): when `sql.contention.txn_id_cache.max_size`
is set to 0, it would effectively turn off transaction ID cache.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants