server: misc cleanup #105448
Assignees
Labels
C-cleanup
Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior.
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
T-sql-foundations
SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Comments
ecwall
added
C-enhancement
craig bot
pushed a commit
that referenced
this issue
Jul 3, 2023
105450: server: inline `conn.checkMaxConnections()`, fix max connection race condition r=rafiss a=ecwall Informs #105448 Inlines `conn.checkMaxConnections()` into `conn.processCommandsAsync()` and fixes a race condition in `conn.checkMaxConnections()` between reading nonRootConnectionCount and updating connectionCount. Co-authored-by: Evan Wall <[email protected]>
This was referenced Jul 3, 2023
craig bot
pushed a commit
that referenced
this issue
Jul 7, 2023
106072: server: close net.Conn unconditionally after accepting r=rafiss,knz a=ecwall Informs #105448 Previously after the server accepted a connection, it was closed in multiple paths and in multiple layers after it was done being used. Now it is always closed in the same layer that accepted it after the serveConn callback returns. Release note: None 106238: server: fix crdb_internal.cluster_inflight_traces in shared process mode r=yuzefovich a=yuzefovich This commit fixes a panic that would previously occur when querying `crdb_internal.cluster_inflight_traces` virtual table when running in shared-process multi-tenant mode. In particular, the problem was that we tried to access node liveness which isn't available, and now we will fall back to the multi-tenant way of doing things (using the instances reader). Additionally, this commit extends the existing test to also run in shared-process multi-tenant config which serves as a regression test for this bug. There is no release note since it's not a user-visible bug. Fixes: #106182. Epic: CRDB-26691 Release note: None 106320: sql: unskip TestSchemaChangeGCJob r=chengxiong-ruan a=chengxiong-ruan Informs: #85876 fixes #60664 A few assumptions has been changed since the test was skipped. For example, the first user defined table descriptor ID and expected error messages. There was actually also bugs in the error assertion in the test for some reason, that is fixed by this PR too. Release note: None 106407: sql: add tests for CTAS, CMVAS with every SHOW statement r=chengxiong-ruan a=ecwall Fixes #105895 Adds tests for CREATE TABLE AS, CREATE MATERIALIZED VIEW AS sourcing from all SHOW statements. Release note: None 106420: server: unskip TestDatabasesTablesV2 r=THardy98 a=THardy98 Resolves: #87074 This test no longer seems flaky after testing (via `./dev test pkg/server --filter=TestDatabasesTablesV2 --stress --stress-args="-p=4" --race --timeout=30m` on gceworker). Release note: None Co-authored-by: Evan Wall <[email protected]> Co-authored-by: Yahor Yuzefovich <[email protected]> Co-authored-by: Chengxiong Ruan <[email protected]> Co-authored-by: Thomas Hardy <[email protected]>
craig bot
pushed a commit
that referenced
this issue
Jul 16, 2023
106066: server: remove unused params r=rafiss a=ecwall Informs #105448 1) Remove unnecessary `conn` method params. 2) Remove unused `conn.sendErr` execCfg param. Co-authored-by: Evan Wall <[email protected]>
craig bot
pushed a commit
that referenced
this issue
Jul 21, 2023
105813: concurrency: re-introduce exclusive lock strength into the lock table r=nvanbenschoten a=arulajmani First 6 commits from #105474 This patch re-introduces exclusive lock strength into the lock table. Now, unreplicated locks are considered to have exclusive lock strength whereas replicated locks have intent lock strength. The testing diff follows from this mapping. This distinction between exclusive lock strength and intent lock strength is not meaningful for serializable transactions by default. As such, this patch doesn't change anything functionally. However, once we plumb in the isolation level of a request in all the right places, this change will be useful. In particular, it'll allow non-locking reads from read committed transactions to not block on exclusive locks. Informs #94729 Release note: None 106943: server: move procCh init into Server.serveImpl r=rafiss a=ecwall Informs #105448 This changes `procCh` to a `sync.WaitGroup` because the channel is never read from and moves initialization into `Server.serveImpl`. Also `processCommandsAsync` is changed to `processCommands` and the goroutine is created inside `Server.serverImpl` to avoid needing a `procCh` parameter. Release note: None 107303: builtins: `crdb_internal.sstable_metrics` fixes r=RahulAggarwal1016 a=RahulAggarwal1016 This pr has the following fixes for the builtin `crdb_internal.sstable_metrics` 1. Remove the extra `,` from `node_id` 2. Change `approximate_span_bytes` to be a `uint64` instead of `[]byte` Next Steps: - Fix `MVCCTimeInterval` display format Informs: #102604 Release-note: None Co-authored-by: Arul Ajmani <[email protected]> Co-authored-by: Evan Wall <[email protected]> Co-authored-by: Rahul Aggarwal <[email protected]>
craig bot
pushed a commit
that referenced
this issue
Aug 4, 2023
107586: sql: remove deferred c.bufferReadyForQuery r=rafiss a=ecwall Informs #105448 This line is only used in a testing knob, but is not required by any tests so it can be removed. Release note: None 108147: ui: upgrade typescript from v4 to v5 r=maryliag a=maryliag Upgrade Typescript from v4 to v5, that has a smaller size, is faster and adds new features. This commit make some updates that were causing errors on the new Typescript version, such as type mismatch error. [1] [1] https://devblogs.microsoft.com/typescript/announcing-typescript-5-0/#forbidden-implicit-coercions-in-relational-operators Epic: none Release note: None 108191: changefeedccl: fix row count in nemeses test r=miretskiy a=jayshrivastava There was a bug in this test where, when using the declarative schema changer, we would increment the running row count by a factor of 3 when doing addColumn/dropColumn operations. This behavior aligns with the legacy schema changer where we expect to see two backfills. For the declarative schema changer, we should only double the row count because we see one backfill. In #108155, this bug was caused the rowcount would be incorrectly nonzero which would cause test would get stuck waiting for rows to be emitted. Fixes: #108155 Release note: None Epic: None Co-authored-by: Evan Wall <[email protected]> Co-authored-by: maryliag <[email protected]> Co-authored-by: Jayant Shrivastava <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Some of the server code could be easier to follow by doing some cleanup
defer close()resources after creationJira issue: CRDB-29051
Epic CRDB-27601
The text was updated successfully, but these errors were encountered: