-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
*: CCL packages imported from non-CCL cockroach binary #91714
Labels
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Comments
ajwerner
added
the
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
label
Nov 10, 2022
ajwerner
added a commit
to ajwerner/cockroach
that referenced
this issue
Dec 14, 2022
Part of cockroachdb#91714 Epic: none Release note: None
ajwerner
added a commit
to ajwerner/cockroach
that referenced
this issue
Dec 14, 2022
This message was relied on by kv code. Nobody used the message at its old path. It's just used for trace events. This PR just moves its code to `kv/bulk/bulkpb` from `ccl/backupccl/backuppb`. Part of cockroachdb#91714 Epic: none Release note: None
ajwerner
added a commit
to ajwerner/cockroach
that referenced
this issue
Dec 14, 2022
Part of cockroachdb#91714 Epic: none Release note: None
craig bot
pushed a commit
that referenced
this issue
Dec 14, 2022
93153: rttanalysis: don't count leasing the database desc r=andreimatei a=andreimatei A bunch of rtt-analysis tests were counting a request for leasing the database descriptor. This is not interesting. This patch makes the test framework lease it first through a "USE" statement. The number of KV requests required for leasing is currently mis-counted. We count 1, but in reality it's 4. A different patch will correct the miscounting that, at which point that would be too significant for the tests. Release note: None Epic: None 93325: multitenant: retain range splits after TRUNCATE for secondary tenants r=knz a=ecwall Fixes #69499 Fixes #82944 Existing split points are preserved after a TRUNCATE statement is executed by a secondary tenant. Release note: None 93354: tracing: disallow children of sterile span with different Tracer r=andreimatei a=andreimatei Before this patch, creating a "child" of a sterile span with a different Tracer than the one used to create the sterile span was tolerated - on the argument that sterile spans don't actually get children (the would-be child span is created as a root), so the arguments for not allowing a children to be created with different tracers don't apply. At the same time, creating a child of a noop span with a different Tracer than the noop span's Tracer was documented to not be allowed. In practice, it was, because the code was confused [1]. This patch disallows creating children of sterile spans with a different tracer, for consistency with all the other spans. The patch also makes it a panic for the children of noop spans to be created with a different Tracer. This is all meant as a cleanup / code simplification. [1] WithParent(sp) meant to treat sterile spans differently than noop spans but it was using sp.IsSterile(), which returns true for both. As such, it was unintentionally returning an empty parent option. startSpanGeneric() meant to check the tracer of parent noop spans, but it was failing to actually do so because it was going through the opts.Parent.empty(). Release note: None Epic: None 93545: sql: make SHOW RANGES FOR TABLE include all indexes r=ajwerner a=knz Informs #80906. Fixes #93546. Fixes #82995. Release note (backward-incompatible change): `SHOW RANGES FOR TABLE` now includes rows for all indexes that support the table. Prior to this change, `SHOW RANGES FOR TABLE foo` was an alias for `SHOW RANGES FOR INDEX foo@primary`. This was causing confusion, as it would miss data for secondary indexes. It is still possible to filter to just the primary index using `SHOW RANGES FOR INDEX foo@primary`. The statement output now also includes the index name. 93557: syntheticprivilegecache: scan all privileges at startup r=ajwerner a=ajwerner #### syntheticprivilegecache: move caching logic out of sql This is a pure refactor to move the logic for caching synthetic privileges from the sql package. This will make it easier to add features later. #### syntheticprivilegecache: scan all privileges at startup Fixes #93182 This commit attempts to alleviate the pain caused by synthetic virtual table privileges introduced in 22.2. We need to fetch privileges for virtual tables to determine whether the user has access. This is done lazily. However, when a user attempts to read a virtual table like pg_class or run SHOW TABLES it will force the privileges to be determined for each virtual table (of which there are 290 at the time of writing). This sequential process can be somewhat slow in a single region cluster and will be very slow in an MR cluster. This patch attempts to somewhat alleviate this pain by scanning the table eagerly during server startup. Release note (performance improvement): In 22.2 we introduced privileges on virtual tables (system catalogs like pg_catalog, information_schema, and crdb_internal). A problem with this new feature is that we now must fetch those privileges into a cache before we can use those tables or determine their visibility in other system catalogs. This process used to occur on-demand, when the privilege was needed. Now we'll fetch these privileges eagerly during startup to mitigate the latency when accessing pg_catalog right after the server boots up. 93563: pgwire: handle decoding Geometry/Geography in binary r=rafiss a=otan Resolves #81066 Resolves #93352 Release note (bug fix): Previously, CockroachDB would error when receiving Geometry/Geography using binary parameters. This is now resolved. 93618: cli,cliccl: move some mt commands to cliccl r=ajwerner a=ajwerner Part of #91714 Epic: none Release note: None Co-authored-by: Andrei Matei <andreimatei1@gmail.com> Co-authored-by: Andrei Matei <andrei@cockroachlabs.com> Co-authored-by: Evan Wall <wall@cockroachlabs.com> Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net> Co-authored-by: Andrew Werner <awerner32@gmail.com> Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
craig bot
pushed a commit
that referenced
this issue
Dec 14, 2022
93610: upgrades: fix desc ID sequence definition to match expectation r=ajwerner a=ajwerner Before this change, the test fails with: ``` Diff: `@@` -29,11 +29,11 `@@` "lastUpdated" TIMESTAMP NOT NULL DEFAULT now():::TIMESTAMP, "valueType" STRING NULL, CONSTRAINT "primary" PRIMARY KEY (name ASC), FAMILY "fam_0_name_value_lastUpdated_valueType" (name, value, "lastUpdated", "valueType") ); -CREATE SEQUENCE public.descriptor_id_seq MINVALUE 1 MAXVALUE 9223372036854775807 INCREMENT 1 START 1; +CREATE SEQUENCE public.descriptor_id_seq MINVALUE 1 MAXVALUE 9223372036854775807 INCREMENT 1 START 104; CREATE TABLE public.tenants ( id INT8 NOT NULL, active BOOL NOT NULL DEFAULT true, info BYTES NULL, name STRING NULL AS (crdb_internal.pb_to_json('cockroach.sql.sqlbase.TenantInfo':::STRING, info)->>'name':::STRING) VIRTUAL, ---- ``` This was revealed by #93487 which fixed the broken roachtest. Expect a few more of these. Fixes: #93602 Release note: None 93619: ui: preserve backwards compatibility for managed-service r=matthewtodd a=matthewtodd These components were renamed in #91860, but the support in managed-service for loading multiple versions of cluster-ui doesn't like it when we do that; it expects changes to be purely additive. We choose here to provide aliases to the old names, rather than undoing the rename completely, so that we can eventually switch managed-service over to the new names once the old versions have fallen out of the support window. Epic: none Release note: None 93622: bulkpb: move backuppb.IngestionPerformanceStats out of ccl r=ajwerner a=ajwerner This message was relied on by kv code. Nobody used the message at its old path. It's just used for trace events. This PR just moves its code to `kv/bulk/bulkpb` from `ccl/backupccl/backuppb`. Part of #91714 Epic: none Release note: None Co-authored-by: Andrew Werner <awerner32@gmail.com> Co-authored-by: Matthew Todd <todd@cockroachlabs.com>
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)
Is your feature request related to a problem? Please describe.
This issue is more severe than #62327. We are actually importing ccl packages into our technically bsl-only binary.
I believe, in practice, there are three deps:
pkg/ccl/backupccl/backuppb
pkg/ccl/streamingccl/streampb
pkg/ccl/sqlproxyccl
(imported bycli
)cliccl
Describe the solution you'd like
We need to add a disallowed imports test to prevent this from happening in the future.
Jira issue: CRDB-21393
The text was updated successfully, but these errors were encountered: