Skip to content

Commit 48b5af5

Browse files
author
marc
committed
storage: use engine for os-level operations.
This is important when encryption is enabled as we need to make sure we handle file metadata properly. While `os.Remove` will leave cruft behind, `os.Link` won't copy the file encryption settings from the original and will therefore make it unreadable. This fixes `RESTORE` which writes file to local disk using local encryption settings then ingests them. The link bypassed the rocksdb Env leaving the ingested file without any encryption settings attached (aka: plaintext). Release note: None
1 parent bfdc5ee commit 48b5af5

File tree

11 files changed

+41
-2
lines changed

11 files changed

+41
-2
lines changed

c-deps/libroach/batch.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,8 @@ DBStatus DBBatch::EnvDeleteFile(DBSlice path) { return FmtStatus("unsupported");
535535

536536
DBStatus DBBatch::EnvDeleteDirAndFiles(DBSlice dir) { return FmtStatus("unsupported"); }
537537

538+
DBStatus DBBatch::EnvLinkFile(DBSlice oldname, DBSlice newname) { return FmtStatus("unsupported"); }
539+
538540
DBWriteOnlyBatch::DBWriteOnlyBatch(DBEngine* db) : DBEngine(db->rep, db->iters), updates(0) {}
539541

540542
DBWriteOnlyBatch::~DBWriteOnlyBatch() {}
@@ -628,6 +630,8 @@ DBStatus DBWriteOnlyBatch::EnvDeleteFile(DBSlice path) { return FmtStatus("unsup
628630

629631
DBStatus DBWriteOnlyBatch::EnvDeleteDirAndFiles(DBSlice dir) { return FmtStatus("unsupported"); }
630632

633+
DBStatus DBWriteOnlyBatch::EnvLinkFile(DBSlice oldname, DBSlice newname) { return FmtStatus("unsupported"); }
634+
631635
rocksdb::WriteBatch::Handler* GetDBBatchInserter(::rocksdb::WriteBatchBase* batch) {
632636
return new DBBatchInserter(batch);
633637
}

c-deps/libroach/batch.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ struct DBBatch : public DBEngine {
4949
virtual DBStatus EnvCloseFile(rocksdb::WritableFile* file);
5050
virtual DBStatus EnvDeleteFile(DBSlice path);
5151
virtual DBStatus EnvDeleteDirAndFiles(DBSlice dir);
52+
virtual DBStatus EnvLinkFile(DBSlice oldname, DBSlice newname);
5253
};
5354

5455
struct DBWriteOnlyBatch : public DBEngine {
@@ -78,6 +79,7 @@ struct DBWriteOnlyBatch : public DBEngine {
7879
virtual DBStatus EnvCloseFile(rocksdb::WritableFile* file);
7980
virtual DBStatus EnvDeleteFile(DBSlice path);
8081
virtual DBStatus EnvDeleteDirAndFiles(DBSlice dir);
82+
virtual DBStatus EnvLinkFile(DBSlice oldname, DBSlice newname);
8183
};
8284

8385
// GetDBBatchInserter returns a WriteBatch::Handler that operates on a

c-deps/libroach/db.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,8 @@ DBStatus DBEnvDeleteFile(DBEngine* db, DBSlice path) { return db->EnvDeleteFile(
477477

478478
DBStatus DBEnvDeleteDirAndFiles(DBEngine* db, DBSlice dir) { return db->EnvDeleteDirAndFiles(dir); }
479479

480+
DBStatus DBEnvLinkFile(DBEngine* db, DBSlice oldname, DBSlice newname) { return db->EnvLinkFile(oldname, newname); }
481+
480482
DBIterator* DBNewIter(DBEngine* db, bool prefix, bool stats) {
481483
rocksdb::ReadOptions opts;
482484
opts.prefix_same_as_start = prefix;

c-deps/libroach/engine.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,4 +435,9 @@ DBStatus DBImpl::EnvDeleteDirAndFiles(DBSlice dir) {
435435
return ToDBStatus(status);
436436
}
437437

438+
// EnvLinkFile creates 'newname' as a hard link to 'oldname'.
439+
DBStatus DBImpl::EnvLinkFile(DBSlice oldname, DBSlice newname) {
440+
return ToDBStatus(this->rep->GetEnv()->LinkFile(ToString(oldname), ToString(newname)));
441+
}
442+
438443
} // namespace cockroach

c-deps/libroach/engine.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ struct DBEngine {
5050
virtual DBStatus EnvCloseFile(rocksdb::WritableFile* file) = 0;
5151
virtual DBStatus EnvDeleteFile(DBSlice path) = 0;
5252
virtual DBStatus EnvDeleteDirAndFiles(DBSlice dir) = 0;
53+
virtual DBStatus EnvLinkFile(DBSlice oldname, DBSlice newname) = 0;
5354

5455
DBSSTable* GetSSTables(int* n);
5556
DBString GetUserProperties();
@@ -94,6 +95,7 @@ struct DBImpl : public DBEngine {
9495
virtual DBStatus EnvCloseFile(rocksdb::WritableFile* file);
9596
virtual DBStatus EnvDeleteFile(DBSlice path);
9697
virtual DBStatus EnvDeleteDirAndFiles(DBSlice dir);
98+
virtual DBStatus EnvLinkFile(DBSlice oldname, DBSlice newname);
9799
};
98100

99101
} // namespace cockroach

c-deps/libroach/include/libroach.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,9 @@ DBStatus DBEnvDeleteFile(DBEngine* db, DBSlice path);
402402
// files it contains but not subdirectories in the given engine.
403403
DBStatus DBEnvDeleteDirAndFiles(DBEngine* db, DBSlice dir);
404404

405+
// DBEnvLinkFile creates 'newname' as a hard link to 'oldname using the given engine.
406+
DBStatus DBEnvLinkFile(DBEngine* db, DBSlice oldname, DBSlice newname);
407+
405408
// DBFileLock contains various parameters set during DBLockFile and required for DBUnlockFile.
406409
typedef void* DBFileLock;
407410

c-deps/libroach/snapshot.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,4 +79,6 @@ DBStatus DBSnapshot::EnvDeleteFile(DBSlice path) { return FmtStatus("unsupported
7979

8080
DBStatus DBSnapshot::EnvDeleteDirAndFiles(DBSlice dir) { return FmtStatus("unsupported"); }
8181

82+
DBStatus DBSnapshot::EnvLinkFile(DBSlice oldname, DBSlice newname) { return FmtStatus("unsupported"); }
83+
8284
} // namespace cockroach

c-deps/libroach/snapshot.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ struct DBSnapshot : public DBEngine {
4646
virtual DBStatus EnvCloseFile(rocksdb::WritableFile* file);
4747
virtual DBStatus EnvDeleteFile(DBSlice path);
4848
virtual DBStatus EnvDeleteDirAndFiles(DBSlice dir);
49+
virtual DBStatus EnvLinkFile(DBSlice oldname, DBSlice newname);
4950
};
5051

5152
} // namespace cockroach

pkg/storage/engine/engine.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,10 @@ type Engine interface {
290290
// not subdirectories from this RocksDB's env. If dir does not exist,
291291
// DeleteDirAndFiles returns nil (no error).
292292
DeleteDirAndFiles(dir string) error
293+
// LinkFile creates 'newname' as a hard link to 'oldname'. This is done using
294+
// the engine implementation. For RocksDB, this means using the Env responsible for the file
295+
// which may handle extra logic (eg: copy encryption settings for EncryptedEnv).
296+
LinkFile(oldname, newname string) error
293297
}
294298

295299
// WithSSTables extends the Engine interface with a method to get info

pkg/storage/engine/rocksdb.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2713,6 +2713,20 @@ func (r *RocksDB) DeleteDirAndFiles(dir string) error {
27132713
return nil
27142714
}
27152715

2716+
// LinkFile creates 'newname' as a hard link to 'oldname'. This use the Env responsible for the file
2717+
// which may handle extra logic (eg: copy encryption settings for EncryptedEnv).
2718+
func (r *RocksDB) LinkFile(oldname, newname string) error {
2719+
if err := statusToError(C.DBEnvLinkFile(r.rdb, goToCSlice([]byte(oldname)), goToCSlice([]byte(newname)))); err != nil {
2720+
return &os.LinkError{
2721+
Op: "link",
2722+
Old: oldname,
2723+
New: newname,
2724+
Err: err,
2725+
}
2726+
}
2727+
return nil
2728+
}
2729+
27162730
// IsValidSplitKey returns whether the key is a valid split key. Certain key
27172731
// ranges cannot be split (the meta1 span and the system DB span); split keys
27182732
// chosen within any of these ranges are considered invalid. And a split key

0 commit comments

Comments
 (0)