Skip to content

Commit a4e6af0

Browse files
ChrisspyBChrisspyB
committed
FDB-508 minor refactor of fdb wipe internals
FDB-508 Fix merge of daos wipe changes
1 parent b6ceb1e commit a4e6af0

25 files changed

+150
-172
lines changed

‎src/fdb5/api/RemoteFDB.cc‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,12 +118,10 @@ struct InspectHelper : BaseAPIHelper<fdb5::ListElement, fdb5::remote::Message::I
118118

119119
struct WipeHelper : BaseAPIHelper<fdb5::CatalogueWipeState, fdb5::remote::Message::Wipe> {
120120

121-
WipeHelper(bool doit, bool porcelain, bool unsafeWipeAll) :
122-
doit_(doit), porcelain_(porcelain), unsafeWipeAll_(unsafeWipeAll) {}
121+
WipeHelper(bool doit, bool porcelain, bool unsafeWipeAll) : doit_(doit), unsafeWipeAll_(unsafeWipeAll) {}
123122

124123
void encodeExtra(eckit::Stream& s) const {
125124
s << doit_;
126-
s << porcelain_;
127125
s << unsafeWipeAll_;
128126
}
129127

@@ -134,7 +132,6 @@ struct WipeHelper : BaseAPIHelper<fdb5::CatalogueWipeState, fdb5::remote::Messag
134132
private:
135133

136134
bool doit_;
137-
bool porcelain_;
138135
bool unsafeWipeAll_;
139136
};
140137

‎src/fdb5/api/local/WipeVisitor.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ bool WipeCatalogueVisitor::visitIndex(const Index& index) {
9292
// n.b. If the request is over-specified (i.e. below the index level), nothing will be removed
9393
bool include = index.key().match(indexRequest_);
9494

95-
include = currentCatalogue_->wipeIndex(index, include, catalogueWipeState_);
95+
include = currentCatalogue_->markIndexForWipe(index, include, catalogueWipeState_);
9696

9797
// Enumerate data files
9898
for (auto& dataURI : index.dataURIs()) {
@@ -110,7 +110,7 @@ void WipeCatalogueVisitor::catalogueComplete(const Catalogue& catalogue) {
110110

111111
ASSERT(currentCatalogue_ == &catalogue);
112112

113-
catalogue.wipeFinalise(catalogueWipeState_);
113+
catalogue.finaliseWipeState(catalogueWipeState_);
114114
catalogueWipeState_.sanityCheck();
115115

116116
queue_.emplace(std::move(catalogueWipeState_));

‎src/fdb5/daos/DaosCatalogue.cc‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ CatalogueWipeState DaosCatalogue::wipeInit() const {
190190
return CatalogueWipeState(dbKey_);
191191
}
192192

193-
bool DaosCatalogue::wipeIndex(const Index& index, bool include, CatalogueWipeState& wipeState) const {
193+
bool DaosCatalogue::markIndexForWipe(const Index& index, bool include, CatalogueWipeState& wipeState) const {
194194

195195
fdb5::DaosKeyValueName location{index.location().uri()};
196196
const fdb5::DaosKeyValueName& db_kv = dbKeyValue();
@@ -239,7 +239,7 @@ bool DaosCatalogue::wipeIndex(const Index& index, bool include, CatalogueWipeSta
239239
return include;
240240
}
241241

242-
void DaosCatalogue::wipeFinalise(CatalogueWipeState& wipeState) const {
242+
void DaosCatalogue::finaliseWipeState(CatalogueWipeState& wipeState) const {
243243

244244
// build database KV URI
245245
const fdb5::DaosKeyValueName& db_kv = dbKeyValue();
@@ -291,7 +291,7 @@ void DaosCatalogue::maskIndexEntries(const std::set<Index>& indexes) const {
291291
}
292292
}
293293

294-
bool DaosCatalogue::doWipeUnknown(const std::set<eckit::URI>& unknownURIs) const {
294+
bool DaosCatalogue::doWipeUnknowns(const std::set<eckit::URI>& unknownURIs) const {
295295
for (const auto& uri : unknownURIs) {
296296
fdb5::DaosName name{uri};
297297
ASSERT(name.hasOID());
@@ -304,7 +304,7 @@ bool DaosCatalogue::doWipeUnknown(const std::set<eckit::URI>& unknownURIs) const
304304
return true;
305305
}
306306

307-
bool DaosCatalogue::doWipe(const CatalogueWipeState& wipeState) const {
307+
bool DaosCatalogue::doWipeURIs(const CatalogueWipeState& wipeState) const {
308308
bool wipeAll = wipeState.safeURIs().empty(); // nothing else in the container
309309

310310
/// @note: this will remove the index and axis KVs, plus the database KV if wipeAll
@@ -317,22 +317,21 @@ bool DaosCatalogue::doWipe(const CatalogueWipeState& wipeState) const {
317317
}
318318

319319
if (wipeAll) {
320-
fdb5::DaosName cont{pool_, db_cont_};
321-
emptyDatabases_.insert(cont.URI());
320+
cleanupEmptyDatabase_ = true;
322321
}
323322

324323
return true;
325324
}
326325

327-
void DaosCatalogue::doWipeEmptyDatabases() const {
326+
void DaosCatalogue::doWipeEmptyDatabase() const {
328327

329-
if (emptyDatabases_.size() == 0)
328+
if (!cleanupEmptyDatabase_) {
330329
return;
331-
332-
ASSERT(emptyDatabases_.size() == 1);
330+
}
331+
fdb5::DaosName cont{pool_, db_cont_};
333332

334333
// remove the database container
335-
fdb5::DaosName contName{*(emptyDatabases_.begin())};
334+
fdb5::DaosName contName{pool_, db_cont_};
336335
ASSERT(!contName.hasOID());
337336
if (contName.exists()) {
338337
ASSERT(contName.listOIDs().size() == 0);
@@ -347,7 +346,7 @@ void DaosCatalogue::doWipeEmptyDatabases() const {
347346
rootKv.remove(key);
348347
}
349348

350-
emptyDatabases_.clear();
349+
cleanupEmptyDatabase_ = false;
351350
}
352351

353352
bool DaosCatalogue::doUnsafeFullWipe() const {

‎src/fdb5/daos/DaosCatalogue.h‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ class DaosCatalogue : public CatalogueImpl, public DaosCommon {
4545
void checkUID() const override { NOTIMP; };
4646
bool exists() const override;
4747
void dump(std::ostream& out, bool simple, const eckit::Configuration& conf) const override { NOTIMP; };
48-
// std::vector<eckit::PathName> metadataPaths() const override { NOTIMP; };
4948
const Schema& schema() const override;
5049
const Rule& rule() const override;
5150

@@ -72,11 +71,11 @@ class DaosCatalogue : public CatalogueImpl, public DaosCommon {
7271
void control(const ControlAction& action, const ControlIdentifiers& identifiers) const override {};
7372

7473
CatalogueWipeState wipeInit() const override;
75-
bool wipeIndex(const Index& index, bool include, CatalogueWipeState& wipeState) const override;
76-
void wipeFinalise(CatalogueWipeState& wipeState) const override;
77-
bool doWipeUnknown(const std::set<eckit::URI>& unknownURIs) const override;
78-
bool doWipe(const CatalogueWipeState& wipeState) const override;
79-
void doWipeEmptyDatabases() const override;
74+
bool markIndexForWipe(const Index& index, bool include, CatalogueWipeState& wipeState) const override;
75+
void finaliseWipeState(CatalogueWipeState& wipeState) const override;
76+
bool doWipeUnknowns(const std::set<eckit::URI>& unknownURIs) const override;
77+
bool doWipeURIs(const CatalogueWipeState& wipeState) const override;
78+
void doWipeEmptyDatabase() const override;
8079
bool doUnsafeFullWipe() const override;
8180

8281
protected: // members

‎src/fdb5/daos/DaosStore.cc‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ void DaosStore::remove(const eckit::URI& uri, std::ostream& logAlways, std::ostr
177177
n.destroy();
178178
}
179179

180-
void DaosStore::prepareWipe(StoreWipeState& storeState, bool doit, bool unsafeWipeAll) {
180+
void DaosStore::finaliseWipeState(StoreWipeState& storeState, bool doit, bool unsafeWipeAll) {
181181
// Note: doit and unsafeWipeAll do not affect the preparation of a local daos store wipe.
182182

183183
const std::set<eckit::URI>& dataURIs = storeState.includedDataURIs(); // included according to cat
@@ -221,7 +221,7 @@ void DaosStore::prepareWipe(StoreWipeState& storeState, bool doit, bool unsafeWi
221221
}
222222
}
223223

224-
bool DaosStore::doWipeUnknownContents(const std::set<eckit::URI>& unknownURIs) const {
224+
bool DaosStore::doWipeUnknowns(const std::set<eckit::URI>& unknownURIs) const {
225225
for (const auto& uri : unknownURIs) {
226226
fdb5::DaosName name{uri};
227227
ASSERT(name.hasOID());
@@ -235,7 +235,7 @@ bool DaosStore::doWipeUnknownContents(const std::set<eckit::URI>& unknownURIs) c
235235
return true;
236236
}
237237

238-
bool DaosStore::doWipe(const StoreWipeState& wipeState) const {
238+
bool DaosStore::doWipeURIs(const StoreWipeState& wipeState) const {
239239
bool wipeAll = wipeState.safeURIs().empty();
240240

241241
// for (const auto& uri : wipeState.dataAuxiliaryURIs()) {
@@ -249,29 +249,26 @@ bool DaosStore::doWipe(const StoreWipeState& wipeState) const {
249249
}
250250

251251
if (wipeAll) {
252-
fdb5::DaosName cont{pool_, db_cont_};
253-
emptyDatabases_.insert(cont.URI());
252+
cleanupEmptyDatabase_ = true;
254253
}
255254

256255
return true;
257256
}
258257

259-
void DaosStore::doWipeEmptyDatabases() const {
258+
void DaosStore::doWipeEmptyDatabase() const {
260259

261-
if (emptyDatabases_.size() == 0)
260+
if (!cleanupEmptyDatabase_) {
262261
return;
262+
}
263263

264-
ASSERT(emptyDatabases_.size() == 1);
264+
fdb5::DaosName contName{pool_, db_cont_};
265265

266-
// remove the database container
267-
fdb5::DaosName contName{*(emptyDatabases_.begin())};
268266
ASSERT(!contName.hasOID());
269267
if (contName.exists()) {
270268
ASSERT(contName.listOIDs().size() == 0);
271269
remove(contName.URI(), std::cout, std::cout, true);
272270
}
273271

274-
emptyDatabases_.clear();
275272
}
276273

277274
bool DaosStore::doUnsafeFullWipe() const {

‎src/fdb5/daos/DaosStore.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,10 @@ class DaosStore : public Store, public DaosCommon {
4545
void checkUID() const override { /* nothing to do */ }
4646

4747
/// Wipe-related methods
48-
void prepareWipe(StoreWipeState& storeState, bool doit, bool unsafeWipeAll) override;
49-
bool doWipeUnknownContents(const std::set<eckit::URI>& unknownURIs) const override;
50-
bool doWipe(const StoreWipeState& wipeState) const override;
51-
void doWipeEmptyDatabases() const override;
48+
void finaliseWipeState(StoreWipeState& storeState, bool doit, bool unsafeWipeAll) override;
49+
bool doWipeUnknowns(const std::set<eckit::URI>& unknownURIs) const override;
50+
bool doWipeURIs(const StoreWipeState& wipeState) const override;
51+
void doWipeEmptyDatabase() const override;
5252
bool doUnsafeFullWipe() const override;
5353

5454
// DAOS store does not currently support auxiliary objects

‎src/fdb5/database/Catalogue.h‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -109,19 +109,19 @@ class Catalogue {
109109
virtual CatalogueWipeState wipeInit() const = 0;
110110

111111
/// Mark an index as to be wiped (include=true) or preserved (include=false)
112-
virtual bool wipeIndex(const Index& index, bool include, CatalogueWipeState& wipeState) const = 0;
112+
virtual bool markIndexForWipe(const Index& index, bool include, CatalogueWipeState& wipeState) const = 0;
113113

114114
/// Finish populating the wipe state
115-
virtual void wipeFinalise(CatalogueWipeState& wipeState) const = 0;
115+
virtual void finaliseWipeState(CatalogueWipeState& wipeState) const = 0;
116116

117117
/// Delete unknown URIs. Part of an --unsafe-wipe-all operation.
118-
virtual bool doWipeUnknown(const std::set<eckit::URI>& unknownURIs) const = 0;
118+
virtual bool doWipeUnknowns(const std::set<eckit::URI>& unknownURIs) const = 0;
119119

120120
/// Delete URIs marked in the wipe state
121-
virtual bool doWipe(const CatalogueWipeState& wipeState) const = 0;
121+
virtual bool doWipeURIs(const CatalogueWipeState& wipeState) const = 0;
122122

123-
/// Delete empty DBs
124-
virtual void doWipeEmptyDatabases() const = 0;
123+
/// At the end of a wipe operation, clean up the DB if it is now empty
124+
virtual void doWipeEmptyDatabase() const = 0;
125125

126126
/// Delete full DB in a single or a few operations
127127
virtual bool doUnsafeFullWipe() const = 0;
@@ -134,7 +134,7 @@ class Catalogue {
134134

135135
protected: // members
136136

137-
mutable std::set<eckit::URI> emptyDatabases_;
137+
mutable bool cleanupEmptyDatabase_ = false;
138138
};
139139

140140
class CatalogueImpl : virtual public Catalogue {

‎src/fdb5/database/Store.h‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -76,25 +76,23 @@ class Store {
7676
/// Wipe-related methods
7777

7878
/// Given a StoreWipeState from the Catalogue, identify URIs to be wiped
79-
virtual void prepareWipe(StoreWipeState& storeState, bool doit, bool unsafeWipeAll) = 0;
79+
virtual void finaliseWipeState(StoreWipeState& storeState, bool doit, bool unsafeWipeAll) = 0;
8080

8181
/// Delete unknown URIs. Part of an --unsafe-wipe-all operation.
82-
virtual bool doWipeUnknownContents(const std::set<eckit::URI>& unknownURIs) const = 0;
82+
virtual bool doWipeUnknowns(const std::set<eckit::URI>& unknownURIs) const = 0;
8383

8484
/// Delete URIs marked in the wipe state
85-
virtual bool doWipe(const StoreWipeState& wipeState) const = 0;
85+
virtual bool doWipeURIs(const StoreWipeState& wipeState) const = 0;
8686

8787
/// Delete empty DBs
88-
virtual void doWipeEmptyDatabases() const = 0;
88+
virtual void doWipeEmptyDatabase() const = 0;
8989

9090
/// Delete full DB in a single or a few operations
9191
virtual bool doUnsafeFullWipe() const = 0;
9292

9393
protected:
9494

95-
/// @todo: Why is this a set and not a single URI?
96-
/// Databases that were found to be empty during wipe, to be removed by wipeEmptyDatabases
97-
mutable std::set<eckit::URI> emptyDatabases_;
95+
mutable bool cleanupEmptyDatabase_ = false;
9896
};
9997

10098

‎src/fdb5/database/WipeCoordinator.cc‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ WipeElements WipeCoordinator::wipe(CatalogueWipeState& catalogueWipeState, bool
2626

2727
// Contact each of the relevant stores.
2828
for (const auto& [storeURI, storeState] : storeWipeStates) {
29-
storeState->store(config_).prepareWipe(*storeState, doit, unsafeWipeAll);
29+
storeState->store(config_).finaliseWipeState(*storeState, doit, unsafeWipeAll);
3030
}
3131

3232
UnknownsBuckets unknownURIs = gatherUnknowns(catalogueWipeState, storeWipeStates);
@@ -54,7 +54,7 @@ WipeElements WipeCoordinator::wipe(CatalogueWipeState& catalogueWipeState, bool
5454
}
5555

5656
if (doit && (!unclean || unsafeWipeAll)) {
57-
doWipe(catalogueWipeState, storeWipeStates, unknownURIs, unsafeWipeAll);
57+
doWipeURIs(catalogueWipeState, storeWipeStates, unknownURIs, unsafeWipeAll);
5858
}
5959

6060
LOG_DEBUG_LIB(LibFdb5) << "WipeCoordinator::wipe - completed" << std::endl;
@@ -139,7 +139,7 @@ WipeElements WipeCoordinator::generateWipeElements(
139139
return report;
140140
}
141141

142-
void WipeCoordinator::doWipe(const CatalogueWipeState& catalogueWipeState,
142+
void WipeCoordinator::doWipeURIs(const CatalogueWipeState& catalogueWipeState,
143143
const std::map<eckit::URI, std::unique_ptr<StoreWipeState>>& storeWipeStates,
144144
const UnknownsBuckets& unknownBuckets, bool unsafeWipeAll) const {
145145

@@ -185,7 +185,7 @@ void WipeCoordinator::doWipe(const CatalogueWipeState& catalogueWipeState,
185185
// 3. Wipe unknown files if unsafeWipeAll
186186
if (unsafeWipeAll) {
187187
LOG_DEBUG_LIB(LibFdb5) << "WipeCoordinator::wipe - wiping unknown URIs" << std::endl;
188-
catalogue->doWipeUnknown(unknownBuckets.catalogue);
188+
catalogue->doWipeUnknowns(unknownBuckets.catalogue);
189189

190190
for (const auto& [uri, storeState] : storeWipeStates) {
191191
if (storeWiped[uri])
@@ -195,7 +195,7 @@ void WipeCoordinator::doWipe(const CatalogueWipeState& catalogueWipeState,
195195

196196
auto it = unknownBuckets.store.find(uri);
197197
if (it != unknownBuckets.store.end()) {
198-
store.doWipeUnknownContents(it->second);
198+
store.doWipeUnknowns(it->second);
199199
}
200200
}
201201
}
@@ -206,21 +206,21 @@ void WipeCoordinator::doWipe(const CatalogueWipeState& catalogueWipeState,
206206
if (storeWiped[uri])
207207
continue;
208208
const Store& store = storeState->store(config_);
209-
store.doWipe(*storeState);
209+
store.doWipeURIs(*storeState);
210210
}
211211

212212
// 5. Wipe files known by the catalogue
213213
LOG_DEBUG_LIB(LibFdb5) << "WipeCoordinator::wipe - wiping catalogue known URIs" << std::endl;
214-
catalogue->doWipe(catalogueWipeState);
214+
catalogue->doWipeURIs(catalogueWipeState);
215215

216216
// 6. wipe empty databases
217217
LOG_DEBUG_LIB(LibFdb5) << "WipeCoordinator::wipe - wiping empty databases" << std::endl;
218-
catalogue->doWipeEmptyDatabases();
218+
catalogue->doWipeEmptyDatabase();
219219
for (const auto& [uri, storeState] : storeWipeStates) {
220220
if (storeWiped[uri])
221221
continue;
222222
const Store& store = storeState->store(config_);
223-
store.doWipeEmptyDatabases();
223+
store.doWipeEmptyDatabase();
224224
}
225225
}
226226

‎src/fdb5/database/WipeCoordinator.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class WipeCoordinator {
4848
const UnknownsBuckets& unknownURIs, bool unsafeWipeAll) const;
4949

5050
/// Delete the data as per the wipe states.
51-
void doWipe(const CatalogueWipeState& catalogueWipeState,
51+
void doWipeURIs(const CatalogueWipeState& catalogueWipeState,
5252
const std::map<eckit::URI, std::unique_ptr<StoreWipeState>>& storeWipeStates,
5353
const UnknownsBuckets& unknownURIs, bool unsafeWipeAll) const;
5454

0 commit comments

Comments
 (0)