testutils/testcluster: quiesce servers in parallel #9338
Conversation
|
Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, some commit checks pending. testutils/testcluster/testcluster.go, line 84 at r1 (raw file):
mind going into more details here? I'm looking at Comments from Reviewable |
|
Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, some commit checks pending. testutils/testcluster/testcluster.go, line 84 at r1 (raw file):
|
|
Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, some commit checks pending. testutils/testcluster/testcluster.go, line 84 at r1 (raw file):
|
|
Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful. testutils/testcluster/testcluster.go, line 84 at r1 (raw file):
|
|
The one real reason is waiting on a raft command which can never commit On Tue, Sep 13, 2016, 16:53 Peter Mattis [email protected] wrote:
-- Tobias |
|
Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful. testutils/testcluster/testcluster.go, line 84 at r1 (raw file):
|
Similar to how multiTestContext shuts down, we need to quiesce the per-server stoppers in parallel to avoid deadlock. Helps with TestBasicManualReplication flakiness, but does not fully deflake.
petermattis
force-pushed
the
pmattis/stop-testcluster
branch
from
8555135 to
7eaff2a
Compare
|
Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, some commit checks pending. testutils/testcluster/testcluster.go, line 84 at r1 (raw file):
|
|
Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful. Comments from Reviewable |
59490: add licensing info to README r=bdarnell a=gemma-shay Added a section on licensing info. Links to the new Licensing doc (#9338) will not work yet, so will wait to merge until they are up. Closes cockroachdb/docs#7441 60513: sql: ensure REGIONAL BY ROW statements roundtrip r=ajstorm a=otan Resolves #59362 See individual commits for details. 60552: sql: add descriptor validation on write r=postamar a=postamar Previously, we didn't systematically validate descriptors when they were written. Furthermore, there existed no common method to validate descriptors across all descriptor subtypes This commit adds three methods to the catalog.Descriptor interface: 1. ValidateSelf ( context.Context ) error 2. Validate ( context.Context, catalog.DescGetter ) error 3. ValidateTxnCommit ( context.Context, catalog.DescGetter) error Each performs a subset the checks performed by the next. ValidateSelf contains all checks which can be performed in isolation, Validate also performs all those involving DescGetters (i.e. cross-reference checks) and ValidateTxnCommit also performs checks which should only be done at commit-time. An example of the latter is checking that a table has a primary key: dropping the PK is allowed within a transaction as long as a new PK is subsequently provided before committing. This commit adds new validation calls when writing descriptors: 1. ValidateSelf is called prior to Collection adding a descriptor Put to a kv.Batch. At this point, we want descriptors to be at least internally-consistent, both to catch validation errors early and because it's not possible to do cross-reference checking at this point (changes on FKs for instance involve multiple descriptors). 2. ValidateTxnCommit is called on the descs.Collection's uncommitted descriptors when the corresponding txn is about to commit, just prior to the two-version-invariant check. These validations may be disabled using a new cluster setting: sql.catalog.descs.validate_on_write.enabled Setting this to false makes it possible to corrupt the descriptor state using the crdb_internal.unsafe_* functions. Release note: None 60616: builtins: add builtin to retrieve the payload(s) for a span. r=knz,irfansharif a=angelapwen Part of addressing #55733 The `payloads_for_span` builtin retrieves all payloads for a given span ID, given that the span is part of an active trace. The payloads are returned in JSONB format. If the span is not found, or if the span does not have any payloads, the builtin returns an empty JSON object. With the appropriate usage of this builtin and the `crdb_internal.trace_id` builtin as shown in the `contention_event` logic test, all payloads for the current trace may be surfaced. Release note (sql change): add `payloads_for_span` builtin that takes in a span ID and returns its paylods in JSONB format. If the span is not found, or if the span does not have any payloads, the builtin returns an empty JSON object. 60692: sql: add tests for inverted indexes on virtual columns r=mgartner a=mgartner No code changes were needed to support inverted indexes on virtual columns. Release note: None Co-authored-by: Gemma Shay <[email protected]> Co-authored-by: Oliver Tan <[email protected]> Co-authored-by: Marius Posta <[email protected]> Co-authored-by: angelapwen <[email protected]> Co-authored-by: Marcus Gartner <[email protected]>
Similar to how multiTestContext shuts down, we need to quiesce the
per-server stoppers in parallel to avoid deadlock. Helps with
TestBasicManualReplication flakiness, but does not fully deflake.
This change is