Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Browse files
Browse the repository at this point in the history
…achdb#108075 cockroachdb#108094 cockroachdb#108097 107475: changefeedccl: update nemeses test to use the declarative schema changer r=HonoreDB a=jayshrivastava ### sql: only use public indexes when fingerprinting Previously, running `SHOW EXPERIMENTAL_FINGERPRINTS` on a table in the middle of a schema change would sometimes cause an error due to missing indexes. The root cause of this error is that fingerprinting attempted to read non-public indexes which exist during the schema change: ``` 1 foo_pkey 2 crdb_internal_index_2_name_placeholder 3 crdb_internal_index_3_name_placeholder ``` The fix to this change is a one line change which ensures fingerprinting reads active indexes only. This change also adds a regression test for this scenario. There was a test util method in `changefeedccl` which was useful for this test. This change moves that util method to `schematestutils` so it can be called outside of `changefeedccl`. Epic: none Release note: None --- ### changefeedccl: update nemeses test to use the declarative schema changer This change updates the nemeses test, which asserts the behavior of changefeeds under a randomized DML/DDL workload. Previously, the test would only run with the legacy schema changer. This is not ideal because, by default, the declarative schema changer is used in production. This change makes it so that the declarative schema changer is used 90% of the time instead. Informs: cockroachdb#106906 Epic: none Release note: None 108019: rpc: fix various test bugs r=aliher1911,pavelkalinnikov a=knz Fixes cockroachdb#108008. TLDR: Prior to this patch, multiple unit tests in the `rpc` package were misconfiguring their heartbeat interval and timeout. This patch fixes this. The issue was that there were two separate locations for these tunable parameters: - `RPCHeartbeat{Interval,Timeout}` fields in `ContextOptions` (previously via `*base.Config`, recently as direct fields in `ContextOptions`) - also, two private fields `heartbeat{Interval,Timeout}` The former was copied into the latter during `NewContext()`. Only the latter (the private fields) matter for the actual behavior. Prior to this commit, the tests would call `NewContext()` and then afterwards would override the fields in `ContextOptions`. These overrides were thus ineffective because the value was already copied by that time. In addition to this design bug, there were a couple of additional bugs lurking: the overrides defined by the tests were problematic in some cases. If these overrides had been effective, the tests would break. The problem was discovered after 8d2d2bf merged, because in that commit some of the overrides from broken tests were taken on board and became effective. This started causing flakes. The fix is to remove the ambiguity in configuration and remove the problematic overrides from offending tests. Release note: None Epic: CRDB-28893 108067: sql: unskip TestCancelSchemaChange r=chengxiong-ruan a=chengxiong-ruan Informs cockroachdb#51796 This commit unskip TestCancelSchemaChange by fixing several wrong assumption of schema changer jobs. Also removed some variables which made no effects on the test. Release note: None Release justification: test only change. 108075: storage: fix overwrite RemoteStorage factory bug r=joshimhoff a=joshimhoff Tomorrow AM, I plan to add a unit test that runs an integration test server with in-memory shared storage. If that ends up being difficult, I may back out and test manually with a local cockroach binary. But we should add such a test eventually either way. **storage: fix overwrite RemoteStorage factory bug** As of this commit, if both cfg.SharedStorage and cfg.RemoteStorageFactory are set, CRDB uses cfg.SharedStorage. Note that eventually we will enable using both at the same time, but with the abstractions available today, that is not easy. We prefer cfg.SharedStorage, since the Locator -> Storage mapping contained in it is needed for CRDB to function properly. Release note: None. Epic: None. 108094: timeutil/ptp: fix conversion from seconds to Time r=knz,erikgrinaker a=pavelkalinnikov Epic: none Release note (bug fix): since 22.2.0, using a PTP clock device (enabled by the --clock-device flag) would generate timestamps in the far future. It now generates the correct time. This could cause nodes to crash due to incorrect timestamps, or in the worst case irreversibly advance the cluster's HLC clock into the far future. 108097: ccl/sqlproxyccl: avoid holding onto the lock in ListenForDenied r=JeffSwenson a=jaylim-crl #### ccl/sqlproxyccl: avoid holding onto the lock in ListenForDenied Previously, ListenForDenied would grab the watcher's lock (which is tied to the scope of the proxy) before calling checkConnection. As part of the updated ACL work, checkConnection now will grab a lock when it calls Initialize on a given tenant. Given that checkConnection can be blocking, grabbing onto the global watcher lock isn't ideal as it could hold up new connections, leading to timeout issues. This commit updates ListenForDenied such that the watcher's lock gets released before invoking checkConnection. #### ccl/sqlproxyccl: avoid tenant lookups if we know the type of connection Previously, we were performing a tenant lookup call before checking on the type of connection. This can be unnecessary (e.g. doing a lookup call for the private endpoints ACL, even if we knew that the connection was a public one). This commit addresses that. Release note: None Epic: none Co-authored-by: Jayant Shrivastava <[email protected]> Co-authored-by: Raphael 'kena' Poss <[email protected]> Co-authored-by: Chengxiong Ruan <[email protected]> Co-authored-by: Josh Imhoff <[email protected]> Co-authored-by: Pavel Kalinnikov <[email protected]> Co-authored-by: Jay <[email protected]>
- Loading branch information
