Commit c40343e7 authored by craig[bot]'s avatar craig[bot]

Merge #45928 #48471

45928: roachtest: add a tag for the owner r=andreimatei,nvanbenschoten a=tbg

Automatically add an `owner-<X>` tag to each roachtest. This allows
listing all of the tests owned by a certain owner, for example

    ./bin/roachtest list tag:owner-kv

Furthermore a bit of piping can generate a regexp suitable for usage
in roachdash:

    ./bin/roachtest list tag:owner-kv | \
      grep -v 'skipped:' | \
      grep -Eo '^[^ ]+' | tr '\n' '|'

(You need to cut the trailing `|` and prefix with `.*` to make it work
in roachdash).

Ideally we would also add a Project field to each OwnerMetadata and
assign to that project upon creating an issue. Alas, the GitHub API does
not let you do that:

https://developer.github.com/v3/issues/#create-an-issue

Release note: None

48471: pkg/util: avoid conflicting lock order in log r=tbg a=BurtonQin

This PR fixes #48447 and #48470
https://github.com/cockroachdb/cockroach/blob/cb60e7536f81c0c3eae854e4774c66a01a4f691c/pkg/util/log/exit_override.go#L51-L56
https://github.com/cockroachdb/cockroach/blob/cb60e7536f81c0c3eae854e4774c66a01a4f691c/pkg/util/log/exit_override.go#L78-L82
https://github.com/cockroachdb/cockroach/blob/cb60e7536f81c0c3eae854e4774c66a01a4f691c/pkg/cmd/zerosum/main.go#L440-L443
`loggerT.mu.Lock()` (`l.mu.Lock()` not `logging.mu.Lock()`) is held before calling `f(2, err)`.
`f` is set to an anonymous function calling `c.Close()` by `main()`.
https://github.com/cockroachdb/cockroach/blob/cb60e7536f81c0c3eae854e4774c66a01a4f691c/pkg/cmd/zerosum/main.go#L440-L443
`c.Close()` will call `n.Kill()` (containing `Node.Lock()`) and `Stop()` (containing `Stopper.mu.Lock()`)
https://github.com/cockroachdb/cockroach/blob/cb60e7536f81c0c3eae854e4774c66a01a4f691c/pkg/acceptance/localcluster/cluster.go#L218-L222
But  in other places, `Node.Lock()` and `Stopper.mu.Lock()` are called before `loggerT.mu.Lock()`.
Thus conflicting lock order occurs.
The patch here is to add `l.mu.Unlock()` before calling `f`. 
But there has been an `Unlock()` after calling `exitLocked()` and `outputToStderr()`:
https://github.com/cockroachdb/cockroach/blob/cb60e7536f81c0c3eae854e4774c66a01a4f691c/pkg/util/log/clog.go#L265-L275
So I also add `l.mu.Lock()` after calling `f` to avoid double unlock.
Co-authored-by: default avatarTobias Schottdorf <[email protected]>
Co-authored-by: default avatarBurtonQin <[email protected]>
......@@ -61,6 +61,8 @@ var roachtestOwners = map[Owner]OwnerMetadata{
`unittest`: {},
}
const defaultTag = "default"
type testRegistry struct {
m map[string]*testSpec
// buildVersion is the version of the Cockroach binary that tests will run against.
......@@ -133,6 +135,10 @@ func (r *testRegistry) prepareSpec(spec *testSpec) error {
if _, ok := roachtestOwners[spec.Owner]; !ok {
return fmt.Errorf(`%s: unknown owner [%s]`, spec.Name, spec.Owner)
}
if len(spec.Tags) == 0 {
spec.Tags = []string{defaultTag}
}
spec.Tags = append(spec.Tags, "owner-"+string(spec.Owner))
return nil
}
......@@ -211,8 +217,8 @@ func newFilter(filter []string) *testFilter {
}
if len(tag) == 0 {
tag = []string{"default"}
rawTag = []string{"tag:default"}
tag = []string{defaultTag}
rawTag = []string{"tag:" + defaultTag}
}
makeRE := func(strs []string) *regexp.Regexp {
......
......@@ -79,7 +79,11 @@ func (l *loggerT) exitLocked(err error) {
f := logging.mu.exitOverride.f
logging.mu.Unlock()
if f != nil {
// Avoid conflicting lock order between l.mu and locks in f.
l.mu.Unlock()
f(2, err)
// Avoid double unlock on l.mu.
l.mu.Lock()
} else {
os.Exit(2)
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment