Skip to content
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.

Sign up for GitHub

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

Jump to bottom

kvserver: TestSideloadingSideloadedStorage fails with RocksDB #50226

Closed
andreimatei opened this issue Jun 15, 2020 · 2 comments · Fixed by #50243
Closed

kvserver: TestSideloadingSideloadedStorage fails with RocksDB #50226

andreimatei opened this issue Jun 15, 2020 · 2 comments · Fixed by #50243
Assignees
@jbowens
Labels
A-testing Testing tools and infrastructure C-test-failure Broken test (automatically or manually discovered).

Comments

@andreimatei
Copy link
Contributor

Randomly when I was testracing the whole package:

--- FAIL: TestSideloadingSideloadedStorage (0.07s)
    --- FAIL: TestSideloadingSideloadedStorage/Disk (0.05s)
        replica_sideload_test.go:295: error truncating sideloaded storage: while purging "/tmp/TestSideloadingSideloadedStorage_Disk896201407/sideloading/r0XXXX/r1": IO error: file rmdir: /tmp/TestSideloadingSideloadedStorage_Disk896201407/sideloading/r0XXXX/r1: Directory not empty
            (1) attached stack trace
              | github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*diskSideloadStorage).TruncateTo
              | 	/home/andrei/go/src/github.com/cockroachdb/cockroach/pkg/kv/kvserver/replica_sideload_disk.go:241
              | github.com/cockroachdb/cockroach/pkg/kv/kvserver.testSideloadingSideloadedStorage.func7
              | 	/home/andrei/go/src/github.com/cockroachdb/cockroach/pkg/kv/kvserver/replica_sideload_test.go:289
              | github.com/cockroachdb/cockroach/pkg/kv/kvserver.testSideloadingSideloadedStorage
              | 	/home/andrei/go/src/github.com/cockroachdb/cockroach/pkg/kv/kvserver/replica_sideload_test.go:341
              | github.com/cockroachdb/cockroach/pkg/kv/kvserver.TestSideloadingSideloadedStorage.func2
              | 	/home/andrei/go/src/github.com/cockroachdb/cockroach/pkg/kv/kvserver/replica_sideload_test.go:96
              | testing.tRunner
              | 	/home/andrei/go1.13/go/src/testing/testing.go:909
              | runtime.goexit
              | 	/home/andrei/go1.13/go/src/runtime/asm_amd64.s:1357
            Wraps: (2) 2 safe details enclosed
            Wraps: (3) while purging "/tmp/TestSideloadingSideloadedStorage_Disk896201407/sideloading/r0XXXX/r1"
            Wraps: (4) IO error: file rmdir: /tmp/TestSideloadingSideloadedStorage_Disk896201407/sideloading/r0XXXX/r1: Directory not empty
            Error types: (1) *withstack.withStack (2) *safedetails.withSafeDetails (3) *errutil.withMessage (4) *storage.Error
@andreimatei andreimatei added C-test-failure Broken test (automatically or manually discovered). A-testing Testing tools and infrastructure labels Jun 15, 2020
@andreimatei andreimatei changed the title kvserver: TestSideloadingSideloadedStorage flake kvserver: TestSideloadingSideloadedStorage fails with RocksDB Jun 15, 2020
@andreimatei
Copy link
Contributor Author

This fails consistently under rocksdb (COCKROACH_STORAGE_ENGINE=rocksdb), but works with Pebble. @petermattis will you route?

@andreimatei andreimatei assigned petermattis and unassigned tbg and petermattis Jun 15, 2020
@petermattis petermattis assigned jbowens and unassigned petermattis Jun 15, 2020
@petermattis
Copy link
Collaborator

I would bet this is fallout from @jbowens recent FS cleanups.

@jbowens jbowens mentioned this issue Jun 15, 2020
Merged
craig bot pushed a commit that referenced this issue Jun 15, 2020
@rohany @yuzefovich
@hueypark @knz @petermattis @jbowens
Merge #49284 #50001 #50188 #50205 #50239 #50243
5cff000 
49284: opt: synthesize check constraints on enum columns r=rohany a=rohany

Fixes #49263.

This PR teaches the optimizer how to synthesize check constraints on
columns of an ENUM type, allowing queries like:

```
CREATE TYPE t AS ENUM ('howdy', 'hello');
CREATE TABLE tt (x t, y INT, PRIMARY KEY (x, y));
SELECT x, y FROM tt WHERE y = 2
```

to be planned using constrained spans on the enum values, rather than a
full table scan.

Release note (performance improvement): Allow the optimizer to use enum
information to generate better query plans.

50001: colexec: minor miscellaneous additions r=yuzefovich a=yuzefovich

**colexec: add casts from datum-backed types to bools**

While investigating unrelated test failures, I added this cast, so we
might as well merge it.

Addresses: #48135.

Release note: None

**colexec: add support for Values core with zero rows**

Release note: None

50188: geo/geogfn: implement ST_Azimuth({geometry,geometry}) r=otan a=hueypark

Fixes #48887

Release note (sql change): This PR implement adds the ST_Azimuth({geometry,geometry})

50205: Makefile: ensure redact_safe.md generation is stable r=RichardJCai a=knz

Fixes #50146

On some platforms, the default behavior of `git grep` is to number
the lines (i.e. `-n` is implicitly added).

This patch makes the `-n` explicit and tweaks the sed patterns to
ensure that the behavior is stable.

Release note: None

50239: kv/kvserver: disable the below-raft proto tracking in race builds r=petermattis a=petermattis

The below-raft proto tracking is meant to catch bugs where we
inadvertently starting marshaling a proto below Raft. This tracking is a
source of signficant memory allocations which measurably slow down race
builds. Since we're already doing this tracking in non-race builds,
there is little benefit to also doing it in race builds. Disabling
below-raft proto tracking for race builds reduces the time to run
`testrace` on the `kv/kvserver` package from 605s to 517s on my laptop.

Release note: None

50243: kv/kvserver: fix TestSideloadingSideloadedStorage w/ RocksDB r=jbowens a=jbowens

Fix the TestSideloadingSideloadedStorage test when running with
COCKROACH_STORAGE_ENGINE=rocksdb. The test has always depended on the
exact error message surfaced from os.Remove. In #49717,
diskSideloadStorage was modified to use the engine's RemoveDir operation
rather than interacting with the filesystem directly. Since Pebble uses
os.Remove for its implementation and emulates its error messages in its
MemFS implementation, the exact message comparison continued to succeed.

After #49717, when running with RocksDB, the RocksDB env's error message
was surfaced, with its own context prefixing. This updates the test to
case-insensitively check for 'directory not empty' instead.

Fixes #50226.

Release note: None

Co-authored-by: Rohan Yadav <rohany@alumni.cmu.edu>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: Jaewan Park <jaewan.huey.park@gmail.com>
Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
Co-authored-by: Peter Mattis <petermattis@gmail.com>
Co-authored-by: Jackson Owens <jackson@cockroachlabs.com>
@craig craig bot closed this as completed in 25385d4 Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jbowens jbowens

Labels
A-testing Testing tools and infrastructure C-test-failure Broken test (automatically or manually discovered).
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

kv/kvserver: fix TestSideloadingSideloadedStorage w/ RocksDB jbowens/cockroach
4 participants
@andreimatei @jbowens @tbg @petermattis