testcluster: minor logging improvements#56346
Conversation
| defer wg.Done() | ||
| if s != nil { | ||
| s.Quiesce(context.TODO()) | ||
| ctx := logtags.AddTag(ctx, "n", i) |
There was a problem hiding this comment.
There's no n0. Isn't there a tc.Servers[i].NodeID() you can use?
| // Create a closer that will stop the individual server stoppers when the | ||
| // cluster stopper is stopped. | ||
| tc.stopper.AddCloser(stop.CloserFn(tc.stopServers)) | ||
| tc.stopper.AddCloser(stop.CloserFn(func() { tc.stopServers(context.TODO()) })) |
There was a problem hiding this comment.
this will never become anything but TODO, so might as well .Background() it
29f8a9f to
6cf73a0
Compare
andreimatei
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @tbg)
pkg/testutils/testcluster/testcluster.go, line 101 at r1 (raw file):
Previously, tbg (Tobias Grieger) wrote…
There's no n0. Isn't there a
tc.Servers[i].NodeID()you can use?
done
pkg/testutils/testcluster/testcluster.go, line 288 at r1 (raw file):
Previously, tbg (Tobias Grieger) wrote…
this will never become anything but TODO, so might as well .Background() it
well closer functions should well take contexts. I'll keep the hope alive.
Log when TestCluster quiescing starts, and add a node log tag to each node's quiescing ctx so messages from different nodes can be disambiguated. Release note: None
6cf73a0 to
8949b6e
Compare
andreimatei
left a comment
There was a problem hiding this comment.
bors r+
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @tbg)
55325: ui: Transactions link between Sessions and Statements r=dhartunian a=elkmaster Resolves: #55131, #56244 Release note (admin ui change): Link to the Transactions page is now shown between the Sessions and Statements links in the left hand navigation. This more clearly reflects the hierarchy between the 3 concepts. 56346: testcluster: minor logging improvements r=andreimatei a=andreimatei Log when TestCluster quiescing starts, and add a node log tag to each node's quiescing ctx so messages from different nodes can be disambiguated. Release note: None 56437: cli, ui: dismiss release notes signup banner per environment variable r=knz,dhartunian a=nkodali Previously, the signup banner could only be dismissed manually. For internal testing purposes, this banner is unnecessary. This change provides a way to dismiss the signup banner upon start of a cluster via the cli by setting the environment variable COCKROACH_UI_RELEASE_NOTES_SIGNUP_DISMISSED=true. Resolves #46998 Release note: none 56533: backupccl: add feature flag support for BACKUP, RESTORE r=otan a=angelapwen Follow-up to the RFC at #55778. This addresses the SRE use case mentioned in #51643 — instead of moving forward with a global denylist as the RFC indicated, we are prototyping feature flags via cluster settings to turn on/off requested features. The first part of the prototype will be done on `BACKUP` and `RESTORE` commands. See [this doc](https://docs.google.com/document/d/1nZSdcK7YprL0P4TAuseY-mvlYnd82IaJ_ptAQDoWB6o/edit?) for further details. Note that the logic test under `ccl/backupccl/testdata/backup-restore/feature-flags` can be tested with the command `make test PKG=./pkg/ccl/backupccl TESTS='TestBackupRestoreDataDriven'` — Commit message below — Adds a cluster setting to toggle a feature flag for the BACKUP and RESTORE commands off and on; as well as a broad category for Bulk IO commands. Currently disabling the cluster setting for Bulk IO will only disable BACKUP and RESTORE jobs, but other types may be included in this category in the future.. The feature is being introduced to address a Cockroach Cloud SRE use case: needing to disable certain categories of features in case of cluster failure. Release note (enterprise change): Adds cluster settings to enable/ disable the BACKUP and RESTORE commands. If a user attempts to use these features while they are disabled, an error indicating that the database administrator has disabled the feature is surfaced. Example usage for the database administrator: SET CLUSTER SETTING feature.bulkio.backup.enabled = FALSE; SET CLUSTER SETTING feature.bulkio.backup.enabled = TRUE; SET CLUSTER SETTING feature.bulkio.restore.enabled = FALSE; SET CLUSTER SETTING feature.bulkio.restore.enabled = TRUE; SET CLUSTER SETTING feature.bulkio.enabled = FALSE; SET CLUSTER SETTING feature.bulkio.enabled = TRUE; 56591: ui: fix Overview screen in OSS builds r=dhartunian a=davepacheco Previously, when using OSS builds (created with `make buildoss`), when you loading the DB Console in your browser, you'd get "Page Not Found". The route for the overview page was missing the leading '/'. This bug appears to have been introduced in 722c932. Release note (admin ui change): This fixes a bug where users of the OSS builds of CockroachDB would see "Page Not Found" when loading the Console. 56600: roachpb: remove SetInner in favor of MustSetInner r=nvanbenschoten a=tbg As of a recent commit, `ErrorDetail.SetInner` became unused, and we can switch to a `MustSetInner` pattern for `ErrorDetail`. Since the codegen involved is shared with {Request,Response}Union, those lose the `SetInner` setter as well; we were always asserting on the returned bool there anyway so this isn't changing anything. Release note: None Co-authored-by: Vlad Los <carrott9@gmail.com> Co-authored-by: Andrei Matei <andrei@cockroachlabs.com> Co-authored-by: Namrata Kodali <namrata@cockroachlabs.com> Co-authored-by: angelapwen <angelaw@cockroachlabs.com> Co-authored-by: Joshua M. Clulow <josh@sysmgr.org> Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com>
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build succeeded: |
Log when TestCluster quiescing starts, and add a node log tag to each
node's quiescing ctx so messages from different nodes can be
disambiguated.
Release note: None