-
Notifications
You must be signed in to change notification settings - Fork 4.1k
storage: sideloading bypasses RocksDB env for some operations #31913
Description
I noticed in #31732 (comment) that the sideloaded storage does not perform the TruncateTo operation through the RocksDB env:
cockroach/pkg/storage/replica_sideload_disk.go
Lines 135 to 149 in fa7fb35
| return err | |
| } | |
| var deleted int | |
| for _, match := range matches { | |
| base := filepath.Base(match) | |
| if len(base) < 1 || base[0] != 'i' { | |
| continue | |
| } | |
| base = base[1:] | |
| upToDot := strings.SplitN(base, ".", 2) | |
| i, err := strconv.ParseUint(upToDot[0], 10, 64) | |
| if err != nil { | |
| return errors.Wrapf(err, "while parsing %q during TruncateTo", match) | |
| } | |
| if i >= index { |
This may or may not be a problem, but it's definitely suspicious. Note how Clear just above the snippet does go through extra trouble to use the env.
Perhaps this all works as intended, depending on how the RocksDB env is implemented. It does seem to work when encryption is on, probably because the env really just does the same thing writing directly to the FS would do. With encryption, is that still true, i.e. are files still passed through to the filesystem into the same location (mod scrambling the contents)? I suspect it might be, but then why does Clear go the extra mile through the env?
This really seems like something needs to be investigated and cleaned up.
Note that there's one case where things are definitely broken, namely the one in which you use an in-memory RocksDB instance. In that case, TruncateTo will snoop around on disk, but the env will keep everything in memory (i.e. TruncateTo becomes a no-op and garbage will pile up in-mem).