roachpb,gc,mvcc: add UseClearRange option to GCRequest_GCKey #7

ajwerner · 2020-07-09T04:11:00Z

This commit adds an optimization to massively reduce the overhead of
garbage collecting large numbers of versions. When running garbage
collection, we currently iterate through the store to send (Key, Timestamp)
pairs in a GCRequest to the store to be evaluated and applied. This can
be rather slow when deleting large numbers of keys, particularly due to the
need to paginate. The primary motivation for pagination is to ensure that
we do not create raft commands with deletions that are too large.

In practice, we find that for the tiny values in a sequence, we find that we
can GC around 1800 versions/s with cockroachdb#51184 and around 900 without it (though
note that in that PR the more versions exist, the worse the throughput will
be). This remains abysmally slow. I imagine that using larger batches could
be one approach to increase the throughput, but, as it stands, 256 KiB is
not a tiny raft command.

This instead turns to the ClearRange operation which can delete all of versions
of a key with the replication overhead of just two copies. This approach is
somewhat controversial because, as @petermattis puts it:

We need to be careful about this. Historically, adding many range tombstones
was very bad for performance. I think we resolved most (all?) of those issues,
but I'm still nervous about embracing using range tombstones below the level
of a Range.

Nevertheless, the results are enticing. Rather than pinning a core at full
utilization for minutes just to clear the versions written to a sequence over
the course of a bit more than an hour, we can clear that in ~2 seconds.

Release note (performance improvement): Improved performance of garbage
collection in the face of large numbers of versions.

petermattis

This is straightforward. Perhaps we should add it to the main repo but protect it behind a cluster setting. That would give us time to get comfortable with it and also give us an escape hatch to enable it on user clusters if they experience problems.

55654: xform: reorganize rules and tests for consistency r=RaduBerinde a=mgartner #### xform: organize rules by the type of expressions they match This commit organizes exploration rules by the type of expressions they match. `GenerateZigzagJoins` and `GenerateInvertedZigzagJoins` have been moved to `select.opt`. All other rules were correctly organized. Release note: None #### xform: reorganize tests to match the organization of the rules This commit moves around (and makes some minor necessary changes) to exploration tests so that tests for rules in `foo.opt` can be found in `testdata/foo`. Most of the changes were related to zigzag join rules which moved from `join` to `select`, and some tests for `GenerateConstrainedScans` that have lived in `scan` for years. The goal of this change is to make it easier to find tests for a specific rule and to set a precedent for future tests. Release note: None Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com>

…db#55722 55248: sql: add issue link to ALTER TABLE DROP COLUMN with sql_safe_updates r=lucy-zhang a=ajwerner Explicit transactions with drop column are very dangerous. Inform the user. Touches cockroachdb#51863. Release note (sql change): Added a note to inform users with an issue link when attempting potentially hazardous DROP COLUMN operations when sql_safe_updates is enabled. 55655: xform: organize custom functions into general_funcs.go and scan_funcs.go r=RaduBerinde a=mgartner #### xform: rename custom_funcs.go to general_funcs.go This is in preparation for breaking up the custom functions into multiple files, similar to the organization of normalization custom functions. Release note: None #### xform: move scan-related custom functions to scan_funcs.go This is one step in breaking up the exploration custom functions into files based on the rules they facilitate, similar to the normalization custom functions. Release note: None 55721: roachtest: bump estimated max warehouses of tpccbench/nodes=3/cpu=16 r=nvanbenschoten a=nvanbenschoten Now that we're importing instead of restoring and picking up a number of optimizations along the way, this estimated max warehouse count is too low. This is why we have been flatlining on: https://roachperf.crdb.dev/?filter=&view=tpccbench%2Fnodes%3D3%2Fcpu%3D16&tab=gce 55722: sql: ban renaming table to a new database unless both schemas are public r=lucy-zhang a=lucy-zhang Previously we would seemingly allow renaming tables to a new database with arbitrary source and target schemas without ever actually reassigning the schema ID. This could lead to a corrupted descriptor with an invalid schema ID for the database. This PR fixes the implementation to return an error when renaming a table to a different database unless both the source and target schemas are `public`. Fixes cockroachdb#55710. Release note (bug fix): Previously, changing the parent database and schema of a table using `RENAME` was seemingly permitted but would lead to corruption of the table metadata. Now an error is returned when attempting to rename a table to a different database except in the case where both the source and target schemas are the `public` schema in each database, which continues to be supported. Co-authored-by: Andrew Werner <ajwerner@cockroachlabs.com> Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com> Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com> Co-authored-by: Lucy Zhang <lucy@cockroachlabs.com>

The output of the SPLIT AT command returns keys in the form `/Table/52/1/1` whereas the output of SHOW RANGES omits the prefix, of table id and index id, and displays keys as `/1`. This is a little confusing and making them consistent would make it easier to visually compare. This patch strips the prefix from the keys in the output of SPLIT AT. Release note (sql change): pretty column of SPLIT AT output was changed by stripping prefix. This makes it consistent with output from SHOW RANGES.

Empty (i.e. uninitialized engines) could receive tombstones before being initialized. Initialization checks that the engine is empty (save for the cluster version) and thus failed. Simply don't write tombstones to uninitialized engines, which is fine since by the time the callbacks fire, at least one is initialized anyway, and besides, this mechanism is best effort. The alternatives would have been to either allow tombstones to be present on an engine that is being bootstrapped, or to give the storage the option to defer writing to the engine once it's bootstrapped. Neither seemed worth the extra work. Fixes cockroachdb#55576. Release note: None

55164: ui: extend diagnostics column to allow activate and download reports r=koorosh a=koorosh Resolves cockroachdb#50824 ~~Depends on cockroachdb/admin-ui-components#31 ~~Depends on cockroachdb/yarn-vendored#42 Previously, Statements table had a Diagnostics column which allowed users to request diagnostics reports for the first time and then displayed status for requested report only. As result it wasn't possible to download already generated report or request new one report from the statements table. With current changes, Diagnostics column allows requesting new reports every time when previous reports are generated. Also, it provides a list with links to download previous reports. The main change is to provide a list of available (or requested) reports for every statement (instead of a single, most recent report as it was before). Then extracted `StatementsPage` component (from `admin-ui-components` package) handles all rendering logic for this list of reports. Minor changes: - `WAITING FOR QUERY` status is renamed to `WAITING` for new design - `getDiagnosticsStatus` utility function is reused to reduce code duplication Release note (admin ui change): Diagnostics column (on statements table) has been changed and includes `Activate` button and dropdown list to download completed reports. Also, diagnostics badge status is changed from `WAITING FOR QUERY` to `WAITING` ![Screen Shot 2020-10-01 at 4 55 32 PM](https://user-images.githubusercontent.com/3106437/94915373-77c62c80-04b5-11eb-8fef-4b33db15613b.png) Co-authored-by: Andrii Vorobiov <and.vorobiov@gmail.com>

…tenant Previously, accessing either of theses tables would lead to an internal error. This was caused by defaulting to using a null node ID if the node ID was unavailable, causing a null violation during validation. The rest of internal tables simply use the zero node ID, so this commit fixes crdb_internal.{leases,node_runtime_info} to use this behavior as well. Release note: None

This commit fixes the normalization rule that converts st_distance to st_dwithin or st_dwithinexclusive, which was broken in the case when the use_spheroid parameter was used. Prior to this commit, the rule was assigning the use_spheroid parameter as the 3rd parameter to st_dwithin or st_dwithinexclusive and the distance parameter as the 4th, but that order does not match the function signatures. This commit fixes the issue by assigning distance as the 3rd parameter and use_spheroid as the 4th if it exists. Fixes cockroachdb#55675 Release note (bug fix): Fixed an internal error that could occur during query planning when the use_spheroid parameter was used in the ST_Distance function as part of a filter predicate. For example, `SELECT ... WHERE ST_Distance(geog1, geog2, false) < 10` previously caused an error. This has now been fixed.

55738: sql: fix crdb_internal.{leases,node_runtime_info} when accessed by a tenant r=tbg a=asubiotto Previously, accessing either of theses tables would lead to an internal error. This was caused by defaulting to using a null node ID if the node ID was unavailable, causing a null violation during validation. The rest of internal tables simply use the zero node ID, so this commit fixes crdb_internal.{leases,node_runtime_info} to use this behavior as well. Release note: None Fixes cockroachdb#55701 Co-authored-by: Alfonso Subiotto Marques <alfonso@cockroachlabs.com>

To implement cockroachdb#48775 I will be modifying the schema GC code to add support for destroying tenants data. I notice3d that the current code has redundant repetitions which makes it hard to understand and modify. The logic for computing new deadline and refreshing tables is being repeated in 3 code blocks - I have merged them together. Release note: none.

Currently, doing direct mutations on a RocksDB instance bypasses custom batching / syncing logic that we've built on top of it. This, or something internal to RocksDB, started leading to some bugs when all direct mutations started passing in WriteOptions.sync = true (see cockroachdb#55240 for when this change went in). In this change, direct mutations still commit the batch with sync=true to guarantee WAL syncing, but they go through the batch commit pipeline too, just like the vast majority of operations already do. Fixes cockroachdb#55362. Release note: None.

55543: sql: make SPLIT AT output be consistent with SHOW_RANGES r=ajwerner a=neeral This fixes cockroachdb#24740 The output of the SPLIT AT command returns keys in the form `/Table/52/1/1` whereas the output of SHOW RANGES omits the prefix, of table id and index id, and displays keys as `/1`. This is a little confusing and making them consistent would make it easier to visually compare. This patch strips the prefix from the keys in the output of SPLIT AT. Release note (sql change): pretty column of SPLIT AT output was changed by stripping prefix. This makes it consistent with output from SHOW RANGES. Co-authored-by: neeral <neeral.dodhia@gmail.com>

…db#55737 55567: geo/geomfn: implement ST_MinimumBoundingCircle r=otan a=C0rWin This commit implements ST_MinimumBoundingCircle builtin function to return polygon shape which approximates minimum bounding circle to contain provided geometry. Resolves: cockroachdb#48987 Signed-off-by: Artem Barger <bartem@il.ibm.com> Release note (sql change): Returns polygon shape which approximates minimum bounding circle to contain geometry 55604: kvserver: improve logging tags in multiTestContext r=andreimatei a=andreimatei Release note: None 55616: colbuilder: disable wrapping of changefeed processors r=yuzefovich a=yuzefovich The root of the problem is that `Columnarizer` has buffering behavior - in 20.1 it will be hanging until `coldata.BatchSize()` (1024 by default) rows are emitted by the changefeed. On 20.2 and master due to dynamic batch size behavior it will still be hanging but in a slightly different manner. This is less of a problem on 20.2 and the current master because the vectorized engine will not be used for the changefeed DistSQL flow since the vectorized row count threshold is never met for it (the estimated row count for the plan is 0, so unless a user does `SET vectorize_row_count_threshold=0;` or `SET vectorize=experimental_always;`, we will always use row-by-row engine). In 20.1 the meaning of `vectorize=on` was different - we never looked at the threshold and used the vectorized engine if it was supported. In order to fix this issue we simply refuse to wrap the changefeed processors, so the row-by-row engine will be always used for changefeed flows. Fixes: cockroachdb#55605. Release note (bug fix): The current implementation of changefeeds is incompatible with the vectorized engine, so whenever the latter is being used to run the former, it could hang indefinitely. This is now fixed. Namely, on 20.2 releases this could happen if the user runs `SET vectorize_row_count_threshold=0;`, and on 20.1 releases - if the user runs `SET vectorize=on`. 55737: schemachange: refactor GC job code r=spaskob a=spaskob To implement cockroachdb#48775 I will be modifying the schema GC code to add support for destroying tenants data. I notice3d that the current code has redundant repetitions which makes it hard to understand and modify. The logic for computing new deadline and refreshing tables is being repeated in 3 code blocks - I have merged them together. Release note: none. Co-authored-by: Artem Barger <bartem@il.ibm.com> Co-authored-by: Andrei Matei <andrei@cockroachlabs.com> Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com> Co-authored-by: Spas Bojanov <spas@cockroachlabs.com>

55708: storage: Use batches for direct RocksDB mutations r=itsbilal a=itsbilal Currently, doing direct mutations on a RocksDB instance bypasses custom batching / syncing logic that we've built on top of it. This, or something internal to RocksDB, started leading to some bugs when all direct mutations started passing in WriteOptions.sync = true (see cockroachdb#55240 for when this change went in). In this change, direct mutations still commit the batch with sync=true to guarantee WAL syncing, but they go through the batch commit pipeline too, just like the vast majority of operations already do. Fixes cockroachdb#55362. Release note: None. 55734: server: skip unit'ed engines in tombstone storage r=irfansharif a=tbg Empty (i.e. uninitialized engines) could receive tombstones before being initialized. Initialization checks that the engine is empty (save for the cluster version) and thus failed. Simply don't write tombstones to uninitialized engines, which is fine since by the time the callbacks fire, at least one is initialized anyway, and besides, this mechanism is best effort. The alternatives would have been to either allow tombstones to be present on an engine that is being bootstrapped, or to give the storage the option to defer writing to the engine once it's bootstrapped. Neither seemed worth the extra work. Fixes cockroachdb#55576. Release note: None 55739: opt: fix normalization of st_distance when use_spheroid parameter used r=rytaft a=rytaft This commit fixes the normalization rule that converts `st_distance` to `st_dwithin` or `st_dwithinexclusive`, which was broken in the case when the `use_spheroid` parameter was used. Prior to this commit, the rule was assigning the `use_spheroid` parameter as the 3rd parameter to `st_dwithin` or `st_dwithinexclusive` and the `distance` parameter as the 4th, but that order does not match the function signatures. This commit fixes the issue by assigning `distance` as the 3rd parameter and `use_spheroid` as the 4th if it exists. Fixes cockroachdb#55675 Release note (bug fix): Fixed an internal error that could occur during query planning when the use_spheroid parameter was used in the ST_Distance function as part of a filter predicate. For example, `SELECT ... WHERE ST_Distance(geog1, geog2, false) < 10` previously caused an error. This has now been fixed. Co-authored-by: Bilal Akhtar <bilal@cockroachlabs.com> Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com> Co-authored-by: Rebecca Taft <becca@cockroachlabs.com>

Changes pulled in: - 3ff24428f10792b9a4e23129fca3d9ba47c15004 tool: Add `db checkpoint` command Release note (cli change): Adds `cockroach debug pebble db checkpoint` debug command to easily create a checkpoint without using rocksdb.

Release note: None

55751: vendor: Bump pebble to 3ff24428f10792b9a4e23129fca3d9ba47c15004 r=petermattis a=itsbilal Changes pulled in: - 3ff24428f10792b9a4e23129fca3d9ba47c15004 tool: Add `db checkpoint` command Release note (cli change): Adds `cockroach debug pebble db checkpoint` debug command to easily create a checkpoint without using rocksdb. Co-authored-by: Bilal Akhtar <bilal@cockroachlabs.com>

I noticed that there is no standardized prefix for commit messages and PRs for RFC changes or additions. I would like to add a recommendation to standardize the prefix to `rfc`. Previous commits have used: `rfc`, `rfcs`, `docs`, `docs/rfcs`, `docs/RFCs` and other variants. Release note: None

Closes cockroachdb#55542. This was causing node deaths to be initially ignored in test failures like cockroachdb#55542. The monitor was not watching when the cluster was restarted, so there was no need to inform it of the restart. Because of this, the monitor had an accumulated "death quota" that allowed it to ignore the first few deaths during the actual run of the test. I tested this 5 times without issue.

In v20.1, we increased the default max range size from 64MB to 512MB. However, we only doubled (258b965) the default raft log truncation threshold. This has the potential to exacerbate issues like cockroachdb#37906, because each range will now handle on average 8 times more write load, and will therefore be forced to truncate its log on average 4 times as frequently as it had previously. This commit bumps the default raft log truncation to 16MB. It doesn't go as far as scaling the log truncation threshold by the max range size (either automatically or with a fixed 32MB default) because the cost of an individual log truncation is proportional to the log size and we don't want to make each log truncation significantly more disruptive. 16MB seems like a good middle ground and we know that we have customers already successfully running in production with this value.

Realease note: None

@yuzefovich

55971: build: increase logictest stress timeout to 2h r=tbg a=asubiotto The default 40 minutes timeout is not enough. Release note: None (testing change) Closes cockroachdb#53729 cc @yuzefovich Co-authored-by: Alfonso Subiotto Marques <alfonso@cockroachlabs.com>

tldr: the conversions between "external" strings and internal usernames was unprincipled, and it turns out, incorrect in some cases. This patch cleans this up by introducing a strict conversion API. **Background** CockroachDB currently performs case folding and unicode NFC normalization upon receiving usernames, specifically usernames received from as SQL login principals. Internally, usernames are often—but not always—considered pre-normalized for the purpose of comparisons, privilege checks, role membership checks and the like. Finally, sometimes usernames are reported "back to outside". In error messages, log files etc, but also: - in the SQL syntax produced by SHOW CREATE, SHOW SYNTAX etc. - to generate OS-level file names for exported files. **New API** This patch introduces a new data type `security.SQLUsername`. It is incomparable and non-convertible with the Go `string`. Engineers must now declare intent about how to do the conversion: - `security.MakeSQLUsernameFromUserInput` converts an "outside" string to a username. - `security.MakeSQLUsernameFromPreNormalizedString` promises that its argument has already been previously normalized. - `security.MakeSQLUsernameFromPreNormalizedStringChecked` also checks that its argument has already been previously normalized. To output usernames, the following APIs are also available. - `username.Normalized()` produces the username itself, without decorations. These corresponds to the raw string (after normalization). - `username.SQLIdentifier()` produces the username in valid SQL syntax, so that it can be injected safely in a SQL statement. - `(*tree.FmtCtx).FormatUsername()` takes a username and properly handles quoting and anonymization, like `FormatName()` does for `tree.Name` already. Likewise, conversion from/to protobuf is now regulated, via the new APIs `username.EncodeProto()` and `usernameproto.Decode()`. **Problems being solved** - the usernames "from outside" were normalized sometimes, *but not consistently*: 1. they were in the arguments of CREATE/DROP/ALTER ROLE. This was not changed. 2. they were not consistently converted in `cockroach cert`. This was corrected. 3. they were not in the `cockroach userfile` commands. This has been adjusted with a reference to issue cockroachdb#55389. 4. they are *not* in GRANT/REVOKE. This patch does not change this behavior, but highlights it by spelling out `MakeSQLUsernameFromPreNormalizedString()` in the implementation. 5. ditto for CREATE SCHEMA ... AUTHORIZATION and ALTER ... OWNER TO 6. they were in the argument to `LoginRequest`. This was not changed. 7. they were not in the argument of the other API requests that allow usernames, for example `ListSessions` or `CancelQuery`. This was not changed, but is now documented in the API. - the usernames "inside" were incorrectly directly injected in SQL statements, even though they may contain special characters that create SQL syntax errors. This has been corrected by suitable uses of the new `SQLIdentifier()` method. - There was an outright bug in a call to `CreateGCJobRec` (something about GCing jobs), where a `Description` field was passed in lieu of a username for a `User` field. The implications of this are unclear. **Status after this change** The new API makes it possible to audit exactly where "sensitive" username/string conversion occurs. After this patch, we find the following uses: - `MakeSQLUsernameFromUserInput`: - pgwire user auth - CLI URL parsing - `cockroach userfile` - `cockroach cert` - `(*rpc.SecurityContext).PGURL()` (unsure whether that's a good thing) - CREATE/DROP/ALTER ROLE - when using literal strings as `role_spec` in the SQL grammar - `MakeSQLUsernameFromPreNormalizedString`: - role membership checks inside SQL based on data read from `system` tables. - in GRANT/REVOKE (this is surprising, see above) - `MakeSQLUsernameFromPreNormalizedStringChecked`: - when intepreting the username in API query parameters, for those API documented as using pre-normalized usernames. Release note: None

55398: *: make conversions between `string` and usernames more explicit r=aaron-crl,arulajmani a=knz Informs (but does not fix) cockroachdb#55396 Informs cockroachdb#54696 Informs cockroachdb#55389 tldr: the conversions between "external" strings and internal usernames was unprincipled, and it turns out, incorrect in some cases. This patch cleans this up by introducing a strict conversion API. **Background** CockroachDB currently performs case folding and unicode NFC normalization upon receiving usernames, specifically usernames received from as SQL login principals. Internally, usernames are often—but not always—considered pre-normalized for the purpose of comparisons, privilege checks, role membership checks and the like. Finally, sometimes usernames are reported "back to outside". In error messages, log files etc, but also: - in the SQL syntax produced by SHOW CREATE, SHOW SYNTAX etc. - to generate OS-level file names for exported files. **New API** This patch introduces a new data type `security.SQLUsername`. It is incomparable and non-convertible with the Go `string`. Engineers must now declare intent about how to do the conversion: - `security.MakeSQLUsernameFromUserInput` converts an "outside" string to a username. - `security.MakeSQLUsernameFromPreNormalizedString` promises that its argument has already been previously normalized. - `security.MakeSQLUsernameFromPreNormalizedStringChecked` also checks that its argument has already been previously normalized. To output usernames, the following APIs are also available. - `username.Normalized()` produces the username itself, without decorations. These corresponds to the raw string (after normalization). - `username.SQLIdentifier()` produces the username in valid SQL syntax, so that it can be injected safely in a SQL statement. - `(*tree.FmtCtx).FormatUsername()` takes a username and properly handles quoting and anonymization, like `FormatName()` does for `tree.Name` already. Likewise, conversion from/to protobuf is now regulated, via the new APIs `username.EncodeProto()` and `usernameproto.Decode()`. **Problems being solved** - the usernames "from outside" were normalized sometimes, *but not consistently*: 1. they were in the arguments of CREATE/DROP/ALTER ROLE. This was not changed. 2. they were not consistently converted in `cockroach cert`. This was corrected. 3. they were not in the `cockroach userfile` commands. This has been adjusted with a reference to issue cockroachdb#55389. 4. they are *not* in GRANT/REVOKE. This patch does not change this behavior, but highlights it by spelling out `MakeSQLUsernameFromPreNormalizedString()` in the implementation. 5. ditto for CREATE SCHEMA ... AUTHORIZATION and ALTER ... OWNER TO 6. they were in the argument to `LoginRequest`. This was not changed. 7. they were not in the argument of the other API requests that allow usernames, for example `ListSessions` or `CancelQuery`. This was not changed, but is now documented in the API. - the usernames "inside" were incorrectly directly injected in SQL statements, even though they may contain special characters that create SQL syntax errors. This has been corrected by suitable uses of the new `SQLIdentifier()` method. - There was an outright bug in a call to `CreateGCJobRec` (something about GCing jobs), where a `Description` field was passed in lieu of a username for a `User` field. The implications of this are unclear. **Status after this change** The new API makes it possible to audit exactly where "sensitive" username/string conversion occurs. After this patch, we find the following uses: - `MakeSQLUsernameFromUserInput`: - pgwire user auth - CLI URL parsing - `cockroach userfile` - `cockroach cert` - `(*rpc.SecurityContext).PGURL()` (unsure whether that's a good thing) - CREATE/DROP/ALTER ROLE - when using literal strings as `role_spec` in the SQL grammar - `MakeSQLUsernameFromPreNormalizedString`: - role membership checks inside SQL based on data read from `system` tables. - in GRANT/REVOKE (this is surprising, see above) - `MakeSQLUsernameFromPreNormalizedStringChecked`: - when intepreting the username in API query parameters, for those API documented as using pre-normalized usernames. Release note: None 55986: opt: use paired joins for left semi inverted joins r=sumeerbhola a=sumeerbhola Semi joins using the inverted index used to be converted to an inner inverted join followed by an inner lookup join and a distinct for de-duplication. They are now converted to paired-joins consisting of an inner inverted join followed by a left semi lookup join, which is more efficient. Release note: None 55997: sql: fix object lookup for ALTER TABLE ... RENAME TO ... r=ajwerner a=jayshrivastava Previously, ALTER TABLE schema.table RENAME TO schema.existing_table would fail because the statement would look up the destination table in the public schema only. This commit fixes this behavior by using the correct destination schema. Fixes cockroachdb#55985 Release note: None Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net> Co-authored-by: sumeerbhola <sumeer@cockroachlabs.com> Co-authored-by: Jayant Shrivastava <jayants@cockroachlabs.com>

Previously, we were creating a bound account in the column backfiller which was not being used to track anything. This change removes that. Release note: None

Make it easier for the developer to know how to fix un-generated code by specifying the required commands. Release note: None

55624: kvserver: don't always refuse to take leases when draining r=andreimatei a=andreimatei Before this patch, a draining node would not take any new leases once it's draining. This is a problem in case all replicas of a range are draining at the same time (e.g. when draining a single-node cluster, or when draining multiple nodes at the same time perhaps by mistake) - nobody wants the lease. Particularly because the liveness range is expiration-based (and thus permanently in need of new leases to be granted), this quickly results in nodes failing their liveness. It also becomes more of a problem with cockroachdb#55148, where we start refusing to take the lease on replicas that are not the leader - so if the leader is draining, we deadlock. This patch makes an exception for leaders, which now no longer refuse the lease even when they're draining. The reasonsing being that it's too easy to deadlock otherwise. Release note: None 56004: build: add instructions to rectify code that is not regenerated r=irfansharif a=otan Make it easier for the developer to know how to fix un-generated code by specifying the required commands. Release note: None 56016: backfill: remove unused bound account from column backfiller r=yuzefovich a=adityamaru Previously, we were creating a bound account in the column backfiller which was not being used to track anything. This change removes that. Release note: None Co-authored-by: Andrei Matei <andrei@cockroachlabs.com> Co-authored-by: Oliver Tan <otan@cockroachlabs.com> Co-authored-by: Aditya Maru <adityamaru@gmail.com>

Motivates the next commit. Release note: None

This commit updates the optimizer to prune synthesized `CHECK` constraint columns for `UPDATES` when columns referenced in the constraints are not updated. This may also allow the optimizer to no longer fetch those referenced columns. This should provide a performance benefit for `UDPATE`s to tables with check constraints. Notably, tables that have been given many column families (in order to reduce contention) should see a significant reduction in contention for `UPDATE`s that mutate a subset of column families. Informs cockroachdb#51526 Release note (performance improvement): Previously, all `CHECK` constraints defined on a table would be tested for every `UPDATE` to the table. Now, a check constraint will not be tested for validity when the values of columns it references are not being updated. The referenced columns are no longer fetchecd in cases where they were only fetched to test `CHECK` constraints.

`201auto` option of `vectorize` setting has been removed since it no longer makes sense to keep (it was introduced as an escape hatch for 20.2 release). Note that we don't need to bump the distsql version because of this change since it is backwards compatible - if the gateway is running the old version that has `vectorize=201auto` set, then we will check whether flows for all nodes don't have non-streaming operators and possibly use `off` option on the flow setup request, then the newer version remote node will check the vectorize setting on the request whether it is not `off` and setup the vectorized flow if it is not. Release note (sql change): `201auto` value for `vectorize` session variable and the corresponding cluster setting has been removed.

We introduced the custom StateMachineSetting type in cockroachdb#17216 to power the underlying machinery for version upgrades: SET CLUSTER SETTING version = <major>-<minor>; At the time we left it generalizable enough to make room for future settings with arbitrary internal transitions. For cluster versions this meant only allowing transitions from one major version to the immediate next, all the while making sure that each binary in the cluster was able to activate the targeted version (the proposed version fell within the allowable range ["binary min supported version", "binary version"]). In the three years since we haven't added any state-machine validated settings that fit the bill, and the layers of abstractions to get to validated cluster version updates are one too many for (my) comfort. This PR introduces a new VersionSetting type that is more tightly coupled with the ClusterVersion type itself. VersionSetting uses language that only appropriate for cluster versions, hopefully clarifying things as we go. We'll depend on this clarification in future PRs when we remove the use of gossip in disseminating cluster version bumps. Release note (sql/cli change): The underlying type for the version cluster setting has been changed. Previously it was of an internal type representing "state machine", but now it's simply "version". This has no operational implications, but it does reflect differently in a few spots: - The `Type` column in `cockroach gen settings-list` will now show "version" instead of "custom validation" - The `setting_type` column for `version` in `SHOW CLUSTER SETTINGS` will now show a "v" instead of an "m" - The `valueType` column for `version` in `system.settings` will now show a "v" instead of an "m"

We were previously shoehorning our tracing subsystem into the opentracing framework. This resulted in lots of bending over backwards to no real gain. Move off the interfaces so that we can more easily refactor the code for always-on tracing as well as make it more digestible. This commit is mostly mechanical. A bunch of code around the RPC interceptors was cribbed from the `otgrpc` package. Cleanups are delegated to additional commits. TestFollowerReadsWithStaleDescriptor, skipped temporarily in cockroachdb#55937, passes again due to the rearrangement in `startSpanGeneric` (we were setting the spanID too late, which led to tags being dropped). Release note: None

…db#55994 cockroachdb#56006 55877: rowflow: account for more memory usage of the row buffer r=yuzefovich a=yuzefovich Previously, we accounted for the memory usage of the row buffer from which the rows are pushed from in the routers only when copying the rows from the row container, but we also have another in-memory row buffer that we can move the rows from which was missing the memory accounting. This is now fixed which also deflakes the router disk spill test. Fixes: cockroachdb#55848. Release note: None 55907: sql: remove vectorize=201auto option r=yuzefovich a=yuzefovich `201auto` option of `vectorize` setting has been removed since it no longer makes sense to keep (it was introduced as an escape hatch for 20.2 release). Note that we don't need to bump the distsql version because of this change since it is backwards compatible - if the gateway is running the old version that has `vectorize=201auto` set, then we will check whether flows for all nodes don't have non-streaming operators and possibly use `off` option on the flow setup request, then the newer version remote node will check the vectorize setting on the request whether it is not `off` and setup the vectorized flow if it is not. Release note (sql change): `201auto` value for `vectorize` session variable and the corresponding cluster setting has been removed. 55982: roachtest: use IMPORT for all TPC-C tests r=nvanbenschoten a=nvanbenschoten Fixes cockroachdb#55541. Fixes cockroachdb#55580. In cockroachdb#54880 and cockroachdb#55050, we switched to using IMPORT for most TPC-C tests, in favor of BACKUP, which requires fixtures to be regenerated whenever we changed the workload. In this PR, we switch over the remaining uses of `fixtures load tpcc` to `fixtures import tpcc` to avoid compatibility issues on older release branches. 55994: settings: delete StateMachineSetting, introduce VersionSetting r=irfansharif a=irfansharif We introduced the custom StateMachineSetting type in cockroachdb#17216 to power the underlying machinery for version upgrades: SET CLUSTER SETTING version = <major>-<minor>; At the time we left it generalizable enough to make room for future settings with arbitrary internal transitions. For cluster versions this meant only allowing transitions from one major version to the immediate next, all the while making sure that each binary in the cluster was able to activate the targeted version (the proposed version fell within the allowable range ["binary min supported version", "binary version"]). In the three years since we haven't added any state-machine validated settings that fit the bill, and the layers of abstractions to get to validated cluster version updates are one too many for (my) comfort. This PR introduces a new VersionSetting type that is more tightly coupled with the ClusterVersion type itself. VersionSetting uses language that only appropriate for cluster versions, hopefully clarifying things as we go. We'll depend on this clarification in future PRs when we remove the use of gossip in disseminating cluster version bumps. Release note (sql/cli change): The underlying type for the version cluster setting has been changed. Previously it was of an internal type representing "state machine", but now it's simply "version". This has no operational implications, but it does reflect differently in a few spots: - The `Type` column in `cockroach gen settings-list` will now show "version" instead of "custom validation" - The `setting_type` column for `version` in `SHOW CLUSTER SETTINGS` will now show a "v" instead of an "m" - The `valueType` column for `version` in `system.settings` will now show a "v" instead of an "m" 56006: logictest: deflake a test r=yuzefovich a=yuzefovich We've recently merged a test that in very rare circumstances could produce a float result that differs from the expected one by 1 in the 15th significant digit (after rounding). I believe that could occur, e.g. when the 15th and 16th significant digits were `35`, and we matched the spec of supporting 15 significant digits for floats, yet the rounding makes us return an unexpected result. This commit rounds to the precision of 1 digit less which should make the test non-flaky. Release note: None Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com> Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com> Co-authored-by: irfan sharif <irfanmahmoudsharif@gmail.com>

The CREATE SCHEMA statement was expanded to allow schema names prefixed with database names. For example, the statement `CREATE SCHEMA other_db.schema_name` is valid. Release note (sql change): The CREATE SCHEMA statement was expanded to allow schema names prefixed with database names.

The DROP SCHEMA statement was expanded to allow schema names prefixed with database names. For example, the statement `DROP SCHEMA schema_name, other_db.schema_name CASCADE` is valid. Release note (sql change): The DROP SCHEMA statement was expanded to allow schema names prefixed with database names.

The ALTER SCHEMA statement was expanded to allow the source schema to be prefixed with a database name. For example, `ALTER SCHEMA other_db.schema_name RENAME TO schema_name_1` is valid. Release note (sql change): The ALTER SCHEMA statement was expanded to allow the source schema to be prefixed with a database name.

The statements GRANT ... ON SCHEMA ..., REVOKE ... ON SCHEMA ..., and SHOW GRANTS ON SCHEMA ... were all updated to allow schema names prefixed with database names. Release note (sql change): The statements GRANT ... ON SCHEMA ..., REVOKE ... ON SCHEMA ..., and SHOW GRANTS ON SCHEMA ... were all updated to allow schema names prefixed with database names.

It's an auto-generated file that bazel doesn't yet know how to construct within the sandbox. Before this PR `make bazel-generate` would show spurious diffs on a clean checkout without this file present. Now it no longer will. Unfortunately it also means that successful bazel builds require `reserved_keywords.go` being pre-generated ahead of time (it's not checked-in into the repo). Once Bazel is taught to generate this file however, this will no longer be the case. It was just something that I missed in cockroachdb#55687. Release note: None

55560: backupccl: avoid div-by-zero crash on failed node count r=dt a=dt We've seen a report of a node that crashed due to a divide-by-zero hit during metrics collection, specifically when computing the throughput-per-node by dividing the backup size by node count. Since this is only now used for that metric, make a failure to count nodes a warning only for release builds (and fallback to 1), and make any error while counting, or not counting to more than 0, a returned error in non-release builds. Release note (bug fix): avoid crashing when BACKUP is unable to count the total nodes in the cluster. 55809: roachtest: fix decommission/randomize r=irfansharif a=tbg The test could end up using fully decommissioned nodes for cli commands, which does not work as of cockroachdb#55286. Fixes cockroachdb#55581. Release note: None 56019: lexbase: pin `reserved_keywords.go` within Bazel r=irfansharif a=irfansharif It's an auto-generated file that bazel doesn't yet know how to construct within the sandbox. Before this PR `make bazel-generate` would show spurious diffs on a clean checkout without this file present. Now it no longer will. Unfortunately it also means that successful bazel builds require `reserved_keywords.go` being pre-generated ahead of time (it's not checked-in into the repo). Once Bazel is taught to generate this file however, this will no longer be the case. It was just something that I missed in cockroachdb#55687. Release note: None Co-authored-by: David Taylor <tinystatemachine@gmail.com> Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com> Co-authored-by: irfan sharif <irfanmahmoudsharif@gmail.com>

55647: sql: allow database prefixes for schema names r=ajwerner a=jayshrivastava This PR adds optional database prefixes to schemas for the following ops: - `CREATE SCHEMA ...` - `DROP SCHEMA ...` - `ALTER SCHEMA` (only the source schema can be prefixed to prevent changing the database of a schema using `ALTER SCHEMA thisdb.foo RENAME TO otherdb.bar`) - `GRANT ... ON SCHEMA ...` - `REVOKE ... ON SCHEMA ...` - `SHOW GRANTS ON SCHEMA ...` Schema names in statements such as `ALTER TYPE t SET SCHEMA` or `ALTER TABLE t SET SCHEMA` etc currently do not take database prefixes (this remains unchanged in this PR) to discourage the creation of cross database relations. Discussion notes for this are [here](https://cockroachlabs.atlassian.net/l/c/CRUiuKYu). Resolves cockroachdb#54295 Co-authored-by: Jayant Shrivastava <jayants@cockroachlabs.com>

55845: importccl: lift computed column check to input converter r=dt a=pbardea The current implementation of checking for validating the number of rows for computed columns for in non-IMPORT INTO backups is inefficient. This commit moves the check from being performed on every row to only being performed once per import. Fixes cockroachdb#55842. Release note: None Co-authored-by: Paul Bardea <pbardea@gmail.com>

Release note: None

56031: build: add SQLPARSER_TARGETS to BUILD.bazel keep r=irfansharif a=otan Release note: None Co-authored-by: Oliver Tan <otan@cockroachlabs.com>

Also changes all callers to appropriately set these parameters. Note that MVCCKeyAndIntentsIterKind would always be correct, but where possible, we want to set MVCCKeyIterKind. Release note: None

56007: opt: prune unnecessary check columns r=RaduBerinde a=mgartner This commit updates the optimizer to prune synthesized `CHECK` constraint columns for `UPDATES` when columns referenced in the constraints are not updated. This may also allow the optimizer to no longer fetch those referenced columns. This should provide a performance benefit for `UDPATE`s to tables with check constraints. Notably, tables that have been given many column families (in order to reduce contention) should see a significant reduction in contention for `UPDATE`s that mutate a subset of column families. Informs cockroachdb#51526 Release note (performance improvement): Previously, all `CHECK` constraints defined on a table would be tested for every `UPDATE` to the table. Now, a check constraint will not be tested for validity when the values of columns it references are not being updated. The referenced columns are no longer fetchecd in cases where they were only fetched to test `CHECK` constraints. Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com>

56022: storage: add MVCCIterKind parameter to Reader functions r=sumeerbhola a=sumeerbhola Also changes all callers to try to appropriately set these parameters. Note that MVCCKeyAndIntentsIterKind would always be correct, but where possible, we want to set MVCCKeyIterKind. Release note: None Co-authored-by: sumeerbhola <sumeer@cockroachlabs.com>

56012: tracing: move off opentracing interfaces r=knz,irfansharif a=tbg We were previously shoehorning our tracing subsystem into the opentracing framework. This resulted in lots of bending over backwards to no real gain. Move off the interfaces so that we can more easily refactor the code for always-on tracing as well as make it more digestible. This commit is mostly mechanical. A bunch of code around the RPC interceptors was cribbed from the `otgrpc` package. Cleanups are delegated to additional commits. TestFollowerReadsWithStaleDescriptor, skipped temporarily in cockroachdb#55937, passes again due to the rearrangement in `startSpanGeneric` (we were setting the spanID too late, which led to tags being dropped). Release note: None Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com>

@petermattis

This commit adds an optimization to massively reduce the overhead of garbage collecting large numbers of versions. When running garbage collection, we currently iterate through the store to send (Key, Timestamp) pairs in a GCRequest to the store to be evaluated and applied. This can be rather slow when deleting large numbers of keys, particularly due to the need to paginate. The primary motivation for pagination is to ensure that we do not create raft commands with deletions that are too large. In practice, we find that for the tiny values in a sequence, we find that we can GC around 1800 versions/s with cockroachdb#51184 and around 900 without it (though note that in that PR the more versions exist, the worse the throughput will be). This remains abysmally slow. I imagine that using larger batches could be one approach to increase the throughput, but, as it stands, 256 KiB is not a tiny raft command. This instead turns to the ClearRange operation which can delete all of versions of a key with the replication overhead of just two copies. This approach is somewhat controversial because, as @petermattis puts it: ``` We need to be careful about this. Historically, adding many range tombstones was very bad for performance. I think we resolved most (all?) of those issues, but I'm still nervous about embracing using range tombstones below the level of a Range. ``` Nevertheless, the results are enticing. Rather than pinning a core at full utilization for minutes just to clear the versions written to a sequence over the course of a bit more than an hour, we can clear that in ~2 seconds. The decision to use a ClearRange is controlled by whether an entire batch would be used to clear versions of a single key. This means that there we'll only send a clear range if there are at least `<key size> * <num versions> > 256 KiB`. There's any additional sanity check that the `<num versions>` is greater than 32 in order to prevent issuing lots of `ClearRange` operations when the cluster has gigantic keys. This new functionality is gated behind both a version and an experimental cluster setting. I'm skipping the release note because of the experimental flag. Release note: None

…db#80762 cockroachdb#80773 79911: opt: refactor and test lookup join key column and expr generation r=mgartner a=mgartner #### opt: simplify fetching outer column in CustomFuncs.findComputedColJoinEquality Previously, `CustomFuncs.findComputedColJoinEquality` used `CustomFuncs.OuterCols` to retrieve the outer columns of computed column expressions. `CustomFuncs.OuterCols` returns the cached outer columns in the expression if it is a `memo.ScalarPropsExpr`, and falls back to calculating the outer columns with `memo.BuildSharedProps` otherwise. Computed column expressions are never `memo.ScalarPropsExpr`s, so we use just use `memo.BuildSharedProps` directly. Release note: None #### opt: make RemapCols a method on Factory instead of CustomFuncs Release note: None #### opt: use partial-index-reduced filters when building lookup expressions This commit makes a minor change to `generateLookupJoinsImpl`. Previously, equality filters were extracted from the original `ON` filters. Now they are extracted from filters that have been reduced by partial index implication. This has no effect on behavior because equality filters that reference columns in two tables cannot exist in partial index predicates, so they will never be eliminated during partial index implication. Release note: None #### opt: moves some lookup join generation logic to lookup join package This commit adds a new `lookupjoin` package. Logic for determining the key columns and lookup expressions for lookup joins has been moved to `lookupJoin.ConstraintBuilder`. The code was moved with as few changes as possible, and the behavior does not change in any way. This move will make it easier to test this code in isolation in the future, and allow for further refactoring. Release note: None #### opt: generalize lookupjoin.ConstraintBuilder API This commit makes the lookupjoin.ConstraintBuilder API more general to make unit testing easier in a future commit. Release note: None #### opt: add data-driven tests for lookupjoin.ConstraintBuilder Release note: None #### opt: add lookupjoin.Constraint struct The `lookupjoin.Constraint` struct has been added to encapsulate multiple data structures that represent a strategy for constraining a lookup join. Release note: None 80511: pkg/cloud/azure: Support specifying Azure environments in storage URLs r=adityamaru a=nlowe-sx The Azure Storage cloud provider learned a new parameter, AZURE_ENVIRONMENT, which specifies which azure environment the storage account in question belongs to. This allows cockroach to backup and restore data to Azure Storage Accounts outside the main Azure Public Cloud. For backwards compatibility, this defaults to "AzurePublicCloud" if AZURE_ENVIRONMENT is not specified. Fixes cockroachdb#47163 ## Verification Evidence I spun up a single node cluster: ``` nlowe@nlowe-z4l:~/projects/github/cockroachdb/cockroach [feat/47163-azure-storage-support-multiple-environments L|✚ 2] [🗓 2022-04-22 08:25:49] $ bazel run //pkg/cmd/cockroach:cockroach -- start-single-node --insecure WARNING: Option 'host_javabase' is deprecated WARNING: Option 'javabase' is deprecated WARNING: Option 'host_java_toolchain' is deprecated WARNING: Option 'java_toolchain' is deprecated INFO: Invocation ID: 11504a98-f767-413a-8994-8f92793c2ecf INFO: Analyzed target //pkg/cmd/cockroach:cockroach (0 packages loaded, 0 targets configured). INFO: Found 1 target... Target //pkg/cmd/cockroach:cockroach up-to-date: _bazel/bin/pkg/cmd/cockroach/cockroach_/cockroach INFO: Elapsed time: 0.358s, Critical Path: 0.00s INFO: 1 process: 1 internal. INFO: Build completed successfully, 1 total action INFO: Build completed successfully, 1 total action * * WARNING: ALL SECURITY CONTROLS HAVE BEEN DISABLED! * * This mode is intended for non-production testing only. * * In this mode: * - Your cluster is open to any client that can access any of your IP addresses. * - Intruders with access to your machine or network can observe client-server traffic. * - Intruders can log in without password and read or write any data in the cluster. * - Intruders can consume all your server's resources and cause unavailability. * * * INFO: To start a secure server without mandating TLS for clients, * consider --accept-sql-without-tls instead. For other options, see: * * - https://go.crdb.dev/issue-v/53404/dev * - https://www.cockroachlabs.com/docs/dev/secure-a-cluster.html * * * WARNING: neither --listen-addr nor --advertise-addr was specified. * The server will advertise "nlowe-z4l" to other nodes, is this routable? * * Consider using: * - for local-only servers: --listen-addr=localhost * - for multi-node clusters: --advertise-addr=<host/IP addr> * * CockroachDB node starting at 2022-04-22 15:25:55.461315977 +0000 UTC (took 2.1s) build: CCL unknown @ (go1.17.6) webui: http://nlowe-z4l:8080/ sql: postgresql://root@nlowe-z4l:26257/defaultdb?sslmode=disable sql (JDBC): jdbc:postgresql://nlowe-z4l:26257/defaultdb?sslmode=disable&user=root RPC client flags: /home/nlowe/.cache/bazel/_bazel_nlowe/cf6ed4d0d14c8e474a5c30d572846d8a/execroot/cockroach/bazel-out/k8-fastbuild/bin/pkg/cmd/cockroach/cockroach_/cockroach <client cmd> --host=nlowe-z4l:26257 --insecure logs: /home/nlowe/.cache/bazel/_bazel_nlowe/cf6ed4d0d14c8e474a5c30d572846d8a/execroot/cockroach/bazel-out/k8-fastbuild/bin/pkg/cmd/cockroach/cockroach_/cockroach.runfiles/cockroach/cockroach-data/logs temp dir: /home/nlowe/.cache/bazel/_bazel_nlowe/cf6ed4d0d14c8e474a5c30d572846d8a/execroot/cockroach/bazel-out/k8-fastbuild/bin/pkg/cmd/cockroach/cockroach_/cockroach.runfiles/cockroach/cockroach-data/cockroach-temp4100501952 external I/O path: /home/nlowe/.cache/bazel/_bazel_nlowe/cf6ed4d0d14c8e474a5c30d572846d8a/execroot/cockroach/bazel-out/k8-fastbuild/bin/pkg/cmd/cockroach/cockroach_/cockroach.runfiles/cockroach/cockroach-data/extern store[0]: path=/home/nlowe/.cache/bazel/_bazel_nlowe/cf6ed4d0d14c8e474a5c30d572846d8a/execroot/cockroach/bazel-out/k8-fastbuild/bin/pkg/cmd/cockroach/cockroach_/cockroach.runfiles/cockroach/cockroach-data storage engine: pebble clusterID: bb3942d7-f241-4d26-aa4a-1bd0d6556e4d status: initialized new cluster nodeID: 1 ``` I was then able to view the contents of a backup hosted in an azure government storage account: ``` root@:26257/defaultdb> SELECT DISTINCT object_name FROM [SHOW BACKUP 'azure://container/path/to/backup?AZURE_ACCOUNT_NAME=account&AZURE_ACCOUNT_KEY=***&AZURE_ENVIRONMENT=AzureUSGovernmentCloud'] WHERE object_type = 'database'; object_name ------------------------------------------ example_database ... (17 rows) Time: 5.859632889s ``` Omitting the `AZURE_ENVIRONMENT` parameter, we can see cockroach defaults to the public cloud where my storage account does not exist: ``` root@:26257/defaultdb> SELECT DISTINCT object_name FROM [SHOW BACKUP 'azure://container/path/to/backup?AZURE_ACCOUNT_NAME=account&AZURE_ACCOUNT_KEY=***'] WHERE object_type = 'database'; ERROR: reading previous backup layers: unable to list files for specified blob: Get "https://account.blob.core.windows.net/container?comp=list&delimiter=path%2Fto%2Fbackup&restype=container&timeout=61": dial tcp: lookup account.blob.core.windows.net on 8.8.8.8:53: no such host ``` ## Tests Two new tests are added to verify that the storage account URL is correctly built from the provided Azure Environment name, and that the Environment defaults to the Public Cloud if unspecified for backwards compatibility. I verified the existing tests pass against a government storage account after specifying `AZURE_ENVIRONMENT` as `AzureUSGovernmentCloud` in the backup URL query parameters: ``` nlowe@nlowe-mbp:~/projects/github/cockroachdb/cockroachdb [feat/47163-azure-storage-support-multiple-environments| …3] [🗓 2022-04-22 17:38:26] $ export AZURE_ACCOUNT_NAME=account nlowe@nlowe-mbp:~/projects/github/cockroachdb/cockroachdb [feat/47163-azure-storage-support-multiple-environments| …3] [🗓 2022-04-22 17:38:42] $ export AZURE_ACCOUNT_KEY=*** nlowe@nlowe-mbp:~/projects/github/cockroachdb/cockroachdb [feat/47163-azure-storage-support-multiple-environments| …3] [🗓 2022-04-22 17:39:25] $ export AZURE_CONTAINER=container nlowe@nlowe-mbp:~/projects/github/cockroachdb/cockroachdb [feat/47163-azure-storage-support-multiple-environments| …3] [🗓 2022-04-22 17:39:48] $ export AZURE_ENVIRONMENT=AzureUSGovernmentCloud nlowe@nlowe-mbp:~/projects/github/cockroachdb/cockroachdb [feat/47163-azure-storage-support-multiple-environments| …3] [🗓 2022-04-22 17:40:15] $ bazel test --test_output=streamed --test_arg=-test.v --action_env=AZURE_ACCOUNT_NAME --action_env=AZURE_ACCOUNT_KEY --action_env=AZURE_CONTAINER --action_env=AZURE_ENVIRONMENT //pkg/cloud/azure:azure_test INFO: Invocation ID: aa88a942-f3c7-4df6-bade-8f5f0e18041f WARNING: Streamed test output requested. All tests will be run locally, without sharding, one at a time INFO: Build option --action_env has changed, discarding analysis cache. INFO: Analyzed target //pkg/cloud/azure:azure_test (468 packages loaded, 16382 targets configured). INFO: Found 1 test target... initialized metamorphic constant "span-reuse-rate" with value 28 === RUN TestAzure === RUN TestAzure/simple_round_trip === RUN TestAzure/exceeds-4mb-chunk === RUN TestAzure/exceeds-4mb-chunk/rand-readats === RUN TestAzure/exceeds-4mb-chunk/rand-readats/#00 cloud_test_helpers.go:226: read 3345 of file at 4778744 === RUN TestAzure/exceeds-4mb-chunk/rand-readats/#1 cloud_test_helpers.go:226: read 7228 of file at 226589 === RUN TestAzure/exceeds-4mb-chunk/rand-readats/#2 cloud_test_helpers.go:226: read 634 of file at 256284 === RUN TestAzure/exceeds-4mb-chunk/rand-readats/#3 cloud_test_helpers.go:226: read 7546 of file at 3546208 === RUN TestAzure/exceeds-4mb-chunk/rand-readats/#4 cloud_test_helpers.go:226: read 24123 of file at 4821795 === RUN TestAzure/exceeds-4mb-chunk/rand-readats/#5 cloud_test_helpers.go:226: read 16899 of file at 403428 === RUN TestAzure/exceeds-4mb-chunk/rand-readats/#6 cloud_test_helpers.go:226: read 29467 of file at 4886370 === RUN TestAzure/exceeds-4mb-chunk/rand-readats/#7 cloud_test_helpers.go:226: read 11700 of file at 1876920 === RUN TestAzure/exceeds-4mb-chunk/rand-readats/#8 cloud_test_helpers.go:226: read 2928 of file at 489781 === RUN TestAzure/exceeds-4mb-chunk/rand-readats/cockroachdb#9 cloud_test_helpers.go:226: read 19933 of file at 1483342 === RUN TestAzure/read-single-file-by-uri === RUN TestAzure/write-single-file-by-uri === RUN TestAzure/file-does-not-exist === RUN TestAzure/List === RUN TestAzure/List/root === RUN TestAzure/List/file-slash-numbers-slash === RUN TestAzure/List/root-slash === RUN TestAzure/List/file === RUN TestAzure/List/file-slash === RUN TestAzure/List/slash-f === RUN TestAzure/List/nothing === RUN TestAzure/List/delim-slash-file-slash === RUN TestAzure/List/delim-data --- PASS: TestAzure (34.81s) --- PASS: TestAzure/simple_round_trip (9.66s) --- PASS: TestAzure/exceeds-4mb-chunk (16.45s) --- PASS: TestAzure/exceeds-4mb-chunk/rand-readats (6.41s) --- PASS: TestAzure/exceeds-4mb-chunk/rand-readats/#00 (0.15s) --- PASS: TestAzure/exceeds-4mb-chunk/rand-readats/#1 (0.64s) --- PASS: TestAzure/exceeds-4mb-chunk/rand-readats/#2 (0.65s) --- PASS: TestAzure/exceeds-4mb-chunk/rand-readats/#3 (0.60s) --- PASS: TestAzure/exceeds-4mb-chunk/rand-readats/#4 (0.75s) --- PASS: TestAzure/exceeds-4mb-chunk/rand-readats/#5 (0.80s) --- PASS: TestAzure/exceeds-4mb-chunk/rand-readats/#6 (0.75s) --- PASS: TestAzure/exceeds-4mb-chunk/rand-readats/#7 (0.65s) --- PASS: TestAzure/exceeds-4mb-chunk/rand-readats/#8 (0.65s) --- PASS: TestAzure/exceeds-4mb-chunk/rand-readats/cockroachdb#9 (0.77s) --- PASS: TestAzure/read-single-file-by-uri (0.60s) --- PASS: TestAzure/write-single-file-by-uri (0.60s) --- PASS: TestAzure/file-does-not-exist (1.05s) --- PASS: TestAzure/List (2.40s) --- PASS: TestAzure/List/root (0.30s) --- PASS: TestAzure/List/file-slash-numbers-slash (0.30s) --- PASS: TestAzure/List/root-slash (0.30s) --- PASS: TestAzure/List/file (0.30s) --- PASS: TestAzure/List/file-slash (0.30s) --- PASS: TestAzure/List/slash-f (0.30s) --- PASS: TestAzure/List/nothing (0.15s) --- PASS: TestAzure/List/delim-slash-file-slash (0.15s) --- PASS: TestAzure/List/delim-data (0.30s) === RUN TestAntagonisticAzureRead --- PASS: TestAntagonisticAzureRead (103.90s) === RUN TestParseAzureURL === RUN TestParseAzureURL/Defaults_to_Public_Cloud_when_AZURE_ENVIRONEMNT_unset === RUN TestParseAzureURL/Can_Override_AZURE_ENVIRONMENT --- PASS: TestParseAzureURL (0.00s) --- PASS: TestParseAzureURL/Defaults_to_Public_Cloud_when_AZURE_ENVIRONEMNT_unset (0.00s) --- PASS: TestParseAzureURL/Can_Override_AZURE_ENVIRONMENT (0.00s) === RUN TestMakeAzureStorageURLFromEnvironment === RUN TestMakeAzureStorageURLFromEnvironment/AzurePublicCloud === RUN TestMakeAzureStorageURLFromEnvironment/AzureUSGovernmentCloud --- PASS: TestMakeAzureStorageURLFromEnvironment (0.00s) --- PASS: TestMakeAzureStorageURLFromEnvironment/AzurePublicCloud (0.00s) --- PASS: TestMakeAzureStorageURLFromEnvironment/AzureUSGovernmentCloud (0.00s) PASS Target //pkg/cloud/azure:azure_test up-to-date: _bazel/bin/pkg/cloud/azure/azure_test_/azure_test INFO: Elapsed time: 159.865s, Critical Path: 152.35s INFO: 66 processes: 2 internal, 64 darwin-sandbox. INFO: Build completed successfully, 66 total actions //pkg/cloud/azure:azure_test PASSED in 139.9s INFO: Build completed successfully, 66 total actions ``` 80705: kvclient: fix gRPC stream leak in rangefeed client r=tbg,srosenberg a=erikgrinaker When the DistSender rangefeed client received a `RangeFeedError` message and propagated a retryable error up the stack, it would fail to close the existing gRPC stream, causing stream/goroutine leaks. Release note (bug fix): Fixed a goroutine leak when internal rangefeed clients received certain kinds of retriable errors. 80762: joberror: add ConnectionReset/ConnectionRefused to retryable err allow list r=miretskiy a=adityamaru Bulk jobs will no longer treat `sysutil.IsErrConnectionReset` and `sysutil.IsErrConnectionRefused` as permanent errors. IMPORT, RESTORE and BACKUP will treat this error as transient and retry. Release note: None 80773: backupccl: break dependency to testcluster r=irfansharif a=irfansharif Noticed we were building testing library packages when building CRDB binaries. $ bazel query "somepath(//pkg/cmd/cockroach-short, //pkg/testutils/testcluster)" //pkg/cmd/cockroach-short:cockroach-short //pkg/cmd/cockroach-short:cockroach-short_lib //pkg/ccl:ccl //pkg/ccl/backupccl:backupccl //pkg/testutils/testcluster:testcluster Release note: None Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com> Co-authored-by: Nathan Lowe <nathan.lowe@spacex.com> Co-authored-by: Erik Grinaker <grinaker@cockroachlabs.com> Co-authored-by: Aditya Maru <adityamaru@gmail.com> Co-authored-by: irfan sharif <irfanmahmoudsharif@gmail.com>

ajwerner force-pushed the ajwerner/clear-range-for-gc-of-many-versions branch 2 times, most recently from 944e073 to 38441b9 Compare July 9, 2020 04:44

ajwerner force-pushed the ajwerner/optimize-MVCCGarbageCollect branch from 9a96e1d to 3a1e899 Compare July 9, 2020 04:44

petermattis reviewed Jul 9, 2020

View reviewed changes

ajwerner force-pushed the ajwerner/clear-range-for-gc-of-many-versions branch from 38441b9 to c6b040a Compare July 13, 2020 19:38

ajwerner force-pushed the ajwerner/optimize-MVCCGarbageCollect branch from 3a1e899 to 8e5423b Compare July 13, 2020 19:38

ajwerner force-pushed the ajwerner/clear-range-for-gc-of-many-versions branch 4 times, most recently from 8387d18 to 6801349 Compare July 19, 2020 01:32

craig[bot] and others added 20 commits October 20, 2020 04:45

authors: add Namrata Kodali to authors

c549a36

Release note: None

authors: add amruss to authors

8fbad2a

Realease note: None

craig[bot] and others added 20 commits October 27, 2020 12:43

Merge cockroachdb#55971

25a628f

55971: build: increase logictest stress timeout to 2h r=tbg a=asubiotto The default 40 minutes timeout is not enough. Release note: None (testing change) Closes cockroachdb#53729 cc @yuzefovich Co-authored-by: Alfonso Subiotto Marques <alfonso@cockroachlabs.com>

backfill: remove unused bound account from column backfiller

2c85868

Previously, we were creating a bound account in the column backfiller which was not being used to track anything. This change removes that. Release note: None

build: add instructions to rectify code that is not regenerated

9a94690

Make it easier for the developer to know how to fix un-generated code by specifying the required commands. Release note: None

settings: add todo to strip out StateMachineSetting

3c7bb47

Motivates the next commit. Release note: None

ajwerner force-pushed the ajwerner/clear-range-for-gc-of-many-versions branch from 6801349 to 2b80295 Compare October 27, 2020 21:18

otan and others added 7 commits October 27, 2020 15:45

build: add SQLPARSER_TARGETS to BUILD.bazel keep

da64067

Release note: None

Merge cockroachdb#56031

c5b2cfc

56031: build: add SQLPARSER_TARGETS to BUILD.bazel keep r=irfansharif a=otan Release note: None Co-authored-by: Oliver Tan <otan@cockroachlabs.com>

storage: add MVCCIterKind parameter to Reader functions

e3aa3c0

Also changes all callers to appropriately set these parameters. Note that MVCCKeyAndIntentsIterKind would always be correct, but where possible, we want to set MVCCKeyIterKind. Release note: None

ajwerner force-pushed the ajwerner/clear-range-for-gc-of-many-versions branch from 2b80295 to 39fabe4 Compare October 28, 2020 13:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

roachpb,gc,mvcc: add UseClearRange option to GCRequest_GCKey #7

roachpb,gc,mvcc: add UseClearRange option to GCRequest_GCKey #7

ajwerner commented Jul 9, 2020

petermattis left a comment

roachpb,gc,mvcc: add UseClearRange option to GCRequest_GCKey #7

Are you sure you want to change the base?

roachpb,gc,mvcc: add UseClearRange option to GCRequest_GCKey #7

Conversation

ajwerner commented Jul 9, 2020

petermattis left a comment

Choose a reason for hiding this comment