-
Notifications
You must be signed in to change notification settings - Fork 13
Wipe refactoring + Toc/Remote Wipe #158
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.
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
base: develop
Are you sure you want to change the base?
Changes from 24 commits
0e05001
26679af
6a3e405
91f076d
4f48991
e1db49e
d792902
0a7ac2f
8496aa8
87d4689
0047c81
cc37840
5bdb2ca
ec10a2d
a2e353d
ec1f383
8c827ad
8393aab
b18250f
a31be04
5a031aa
0ec4821
7f8a896
2fca1ef
70e4cda
cabb779
5b1e5d2
32579fb
494321d
f90f3e0
92ff46d
a00d02b
846f3ed
def603e
6788d08
49cb9bf
8cfe905
b3023d0
8103bcc
6b1bcea
e89e31c
7e92f92
f737567
24d98b9
e3debd8
8a7696c
141facb
8a1de33
f6f2f53
1da9aa2
402c4e6
d2a83d7
a736b82
03bb1a0
2ba5fdc
dc94527
f1ddcc9
0438e94
55e8daa
b3055b8
cc3d948
76aff85
a3d0af5
a8073ef
ce0ce86
9866dfe
8ac4d7d
70635b5
6cccc26
155a89e
6ba1aa3
0c5b9ee
714bdd6
aca2d88
03e5a10
6afe3da
2cb1578
1b725bf
92c176e
10c64f3
bd9df13
b98fa39
eba4a21
a86d2a8
188ce86
1f1a8ad
8b1c2cd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| 5.17.3 | ||
| 5.17.4-rc |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| /* | ||
| * (C) Copyright 1996- ECMWF. | ||
| * | ||
| * This software is licensed under the terms of the Apache Licence Version 2.0 | ||
| * which can be obtained at http://www.apache.org/licenses/LICENSE-2.0. | ||
| * In applying this licence, ECMWF does not waive the privileges and immunities | ||
| * granted to it by virtue of its status as an intergovernmental organisation nor | ||
| * does it submit to any jurisdiction. | ||
| */ | ||
|
|
||
| #include "fdb5/api/helpers/WipeIterator.h" | ||
|
|
||
| #include "eckit/log/Log.h" | ||
| #include "eckit/serialisation/Stream.h" | ||
|
|
||
| #include "fdb5/LibFdb5.h" | ||
|
|
||
| using namespace eckit; | ||
| namespace fdb5 { | ||
|
|
||
| //---------------------------------------------------------------------------------------------------------------------- | ||
|
|
||
| std::ostream& operator<<(std::ostream& s, const WipeElementType& t) { | ||
| switch (t) { | ||
| case WipeElementType::WIPE_CATALOGUE_INFO: | ||
| s << "WIPE_CATALOGUE_INFO"; | ||
| break; | ||
| case WipeElementType::WIPE_CATALOGUE: | ||
| s << "WIPE_CATALOGUE"; | ||
| break; | ||
| case WipeElementType::WIPE_CATALOGUE_SAFE: | ||
| s << "WIPE_CATALOGUE_SAFE"; | ||
| break; | ||
| case WipeElementType::WIPE_CATALOGUE_AUX: | ||
| s << "WIPE_CATALOGUE_AUX"; | ||
| break; | ||
| case WipeElementType::WIPE_STORE_INFO: | ||
| s << "WIPE_STORE_INFO"; | ||
| break; | ||
| case WipeElementType::WIPE_STORE: | ||
| s << "WIPE_STORE"; | ||
| break; | ||
| case WipeElementType::WIPE_STORE_URI: | ||
| s << "WIPE_STORE_URI"; | ||
| break; | ||
| case WipeElementType::WIPE_STORE_AUX: | ||
| s << "WIPE_STORE_AUX"; | ||
| break; | ||
| } | ||
| s << "(" << ((int)t) << ")"; | ||
| return s; | ||
| } | ||
|
|
||
| WipeElement::WipeElement(WipeElementType type, const std::string& msg, eckit::URI uri) : | ||
| type_(type), msg_(msg), uris_({uri}) {} | ||
|
|
||
| WipeElement::WipeElement(WipeElementType type, const std::string& msg, const std::vector<eckit::URI>& uris) : | ||
| type_(type), msg_(msg), uris_(uris) {} | ||
|
|
||
| WipeElement::WipeElement(eckit::Stream& s) { | ||
| unsigned char t; | ||
| s >> t; | ||
| type_ = static_cast<WipeElementType>(t); | ||
| s >> msg_; | ||
| size_t numURIs; | ||
| s >> numURIs; | ||
| for (size_t i = 0; i < numURIs; i++) { | ||
| uris_.push_back(s); | ||
| } | ||
| } | ||
|
|
||
| void WipeElement::print(std::ostream& out) const { | ||
|
|
||
| LOG_DEBUG_LIB(LibFdb5) << "Wipe(type=" << type_ << ",msg=" << msg_ << ",uris=["; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oof. This is unpleasant inside ::print(), as this method is typically called in the middle of ostream output. Which results in info/error/debug output becoming interleaved on the terminal. A but unpleasant. |
||
| std::string sep = ""; | ||
| for (const auto& uri : uris_) { | ||
| LOG_DEBUG_LIB(LibFdb5) << sep << std::endl << " " << uri; | ||
| sep = ","; | ||
| } | ||
| LOG_DEBUG_LIB(LibFdb5) << "])" << std::endl; | ||
|
|
||
|
|
||
| out << msg_ << std::endl; | ||
| if (type_ != WIPE_CATALOGUE_INFO && type_ != WIPE_STORE_INFO) { | ||
| if (uris_.size() > 0) { | ||
| for (const auto& uri : uris_) { | ||
| out << " " << uri.asString() << std::endl; | ||
| } | ||
| out << std::endl; | ||
| } | ||
| else { | ||
| out << " - NONE -" << std::endl; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| size_t encodeSizeString(const std::string& s) { | ||
| return (1 + 4 + s.size()); | ||
| } | ||
|
|
||
| size_t WipeElement::encodeSize() const { | ||
| // size_t aux = 0; | ||
| // for (const auto& l : uris_) { | ||
| // aux += 256; | ||
| // } | ||
| return 1 + 4 + encodeSizeString(msg_) + 1 + 4 + (256 * uris_.size()); | ||
| } | ||
|
|
||
| void WipeElement::encode(eckit::Stream& s) const { | ||
| s << static_cast<unsigned char>(type_); | ||
| s << msg_; | ||
| s << uris_.size(); | ||
| for (const auto& uri : uris_) { | ||
| s << uri; | ||
| } | ||
| } | ||
|
|
||
| //---------------------------------------------------------------------------------------------------------------------- | ||
|
|
||
| } // namespace fdb5 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,6 +46,7 @@ list( APPEND fdb5_srcs | |
| api/helpers/LockIterator.h | ||
| api/helpers/MoveIterator.h | ||
| api/helpers/StatusIterator.h | ||
| api/helpers/WipeIterator.cc | ||
| api/helpers/WipeIterator.h | ||
| api/helpers/PurgeIterator.h | ||
| api/helpers/StatsIterator.cc | ||
|
|
@@ -118,8 +119,8 @@ list( APPEND fdb5_srcs | |
| database/Store.h | ||
| database/PurgeVisitor.cc | ||
| database/PurgeVisitor.h | ||
| database/WipeVisitor.cc | ||
| database/WipeVisitor.h | ||
| # database/WipeVisitor.cc | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove |
||
| # database/WipeVisitor.h | ||
| database/MoveVisitor.cc | ||
| database/MoveVisitor.h | ||
| database/IndexAxis.cc | ||
|
|
@@ -318,8 +319,6 @@ if( HAVE_TOCFDB ) | |
| toc/TocPurgeVisitor.h | ||
| toc/TocSerialisationVersion.cc | ||
| toc/TocSerialisationVersion.h | ||
| toc/TocWipeVisitor.cc | ||
| toc/TocWipeVisitor.h | ||
| toc/TocMoveVisitor.cc | ||
| toc/TocMoveVisitor.h | ||
| toc/TocRecord.cc | ||
|
|
@@ -389,8 +388,8 @@ if( HAVE_DAOSFDB ) | |
| daos/DaosIndexLocation.cc | ||
| daos/DaosCommon.h | ||
| daos/DaosCommon.cc | ||
| daos/DaosWipeVisitor.h | ||
| daos/DaosWipeVisitor.cc | ||
| # daos/DaosWipeVisitor.h | ||
| # daos/DaosWipeVisitor.cc | ||
|
Comment on lines
+393
to
+394
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. del |
||
| daos/DaosArrayPartHandle.h | ||
| daos/DaosArrayPartHandle.cc | ||
| daos/DaosLazyFieldLocation.h | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,9 +49,11 @@ struct ListHelper : BaseAPIHelper<fdb5::ListElement, fdb5::remote::Message::List | |
|
|
||
| if (elem.hasLocation()) { | ||
|
|
||
| eckit::Log::debug<fdb5::LibFdb5>() << "ListHelper::valueFromStream - original location: "; | ||
| elem.location().dump(eckit::Log::debug<fdb5::LibFdb5>()); | ||
| eckit::Log::debug<fdb5::LibFdb5>() << std::endl; | ||
| if (fdb5::LibFdb5::instance().debug()) { | ||
| eckit::Log::debug<fdb5::LibFdb5>() << "ListHelper::valueFromStream - original location: "; | ||
| elem.location().dump(eckit::Log::debug<fdb5::LibFdb5>()); | ||
| eckit::Log::debug<fdb5::LibFdb5>() << std::endl; | ||
| } | ||
|
|
||
| // TODO move the endpoint replacement to the server side () | ||
| if (elem.location().uri().scheme() == "fdb") { | ||
|
|
@@ -92,9 +94,11 @@ struct InspectHelper : BaseAPIHelper<fdb5::ListElement, fdb5::remote::Message::I | |
| static fdb5::ListElement valueFromStream(eckit::Stream& s, fdb5::RemoteFDB* fdb) { | ||
| fdb5::ListElement elem(s); | ||
|
|
||
| eckit::Log::debug<fdb5::LibFdb5>() << "InspectHelper::valueFromStream - original location: "; | ||
| elem.location().dump(eckit::Log::debug<fdb5::LibFdb5>()); | ||
| eckit::Log::debug<fdb5::LibFdb5>() << std::endl; | ||
| if (fdb5::LibFdb5::instance().debug()) { | ||
| eckit::Log::debug<fdb5::LibFdb5>() << "InspectHelper::valueFromStream - original location: "; | ||
| elem.location().dump(eckit::Log::debug<fdb5::LibFdb5>()); | ||
| eckit::Log::debug<fdb5::LibFdb5>() << std::endl; | ||
| } | ||
|
|
||
| if (elem.location().uri().scheme() == "fdb") { | ||
| eckit::net::Endpoint fieldLocationEndpoint{elem.location().uri().host(), elem.location().uri().port()}; | ||
|
|
@@ -111,6 +115,29 @@ struct InspectHelper : BaseAPIHelper<fdb5::ListElement, fdb5::remote::Message::I | |
| } | ||
| }; | ||
|
|
||
| struct WipeHelper : BaseAPIHelper<fdb5::WipeElement, fdb5::remote::Message::Wipe> { | ||
|
|
||
| WipeHelper(bool doit, bool porcelain, bool unsafeWipeAll) : | ||
| doit_(doit), porcelain_(porcelain), unsafeWipeAll_(unsafeWipeAll) {} | ||
|
|
||
| void encodeExtra(eckit::Stream& s) const { | ||
| s << doit_; | ||
| s << porcelain_; | ||
| s << unsafeWipeAll_; | ||
| } | ||
|
|
||
| static fdb5::WipeElement valueFromStream(eckit::Stream& s, fdb5::RemoteFDB* fdb) { | ||
| fdb5::WipeElement elem{s}; | ||
| return elem; | ||
| } | ||
|
|
||
| private: | ||
|
|
||
| bool doit_; | ||
| bool porcelain_; | ||
| bool unsafeWipeAll_; | ||
| }; | ||
|
|
||
| } // namespace | ||
|
|
||
| namespace fdb5 { | ||
|
|
@@ -281,6 +308,10 @@ StatsIterator RemoteFDB::stats(const FDBToolRequest& request) { | |
| return forwardApiCall(StatsHelper(), request); | ||
| } | ||
|
|
||
| WipeIterator RemoteFDB::wipe(const FDBToolRequest& request, bool doit, bool porcelain, bool unsafeWipeAll) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My understanding of this is that it implies that wipe runs fully on the Catalogue (none of the communication is via the client). What happens in the case that we have a remote Catalogue, but local stores (e.g. stores on Lustre). The Catalogue may not have the right filesystem permissions to wipe data which has been written by the user. I'm not sure that this design is functionally correct. |
||
| return forwardApiCall(WipeHelper(doit, porcelain, unsafeWipeAll), request); | ||
| } | ||
|
|
||
| void RemoteFDB::print(std::ostream& s) const { | ||
| s << "RemoteFDB(...)"; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ | |
| #ifndef fdb5_ListIterator_H | ||
| #define fdb5_ListIterator_H | ||
|
|
||
| #include <unordered_map> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this need in this header?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. I'm very surprised to see this being added to this header when the list functionality should be essentially untouched here... |
||
| #include <unordered_set> | ||
| #include <utility> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it'd be good to add |
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| /* | ||
| * (C) Copyright 1996- ECMWF. | ||
| * | ||
| * This software is licensed under the terms of the Apache Licence Version 2.0 | ||
| * which can be obtained at http://www.apache.org/licenses/LICENSE-2.0. | ||
| * In applying this licence, ECMWF does not waive the privileges and immunities | ||
| * granted to it by virtue of its status as an intergovernmental organisation nor | ||
| * does it submit to any jurisdiction. | ||
| */ | ||
|
|
||
| #include "fdb5/api/helpers/WipeIterator.h" | ||
|
|
||
| #include "eckit/serialisation/Stream.h" | ||
|
|
||
| namespace fdb5 { | ||
|
|
||
| //---------------------------------------------------------------------------------------------------------------------- | ||
|
|
||
| WipeElement::WipeElement(WipeElementType type, const std::string& msg, eckit::URI uri) : | ||
| type_(type), msg_(msg), uris_({uri}) {} | ||
|
|
||
| WipeElement::WipeElement(WipeElementType type, const std::string& msg) : type_(type), msg_(msg), uris_({}) {} | ||
|
|
||
| WipeElement::WipeElement(WipeElementType type, const std::string& msg, std::set<eckit::URI>&& uris) : | ||
| type_(type), msg_(msg), uris_(std::move(uris)) {} | ||
|
|
||
| WipeElement::WipeElement(eckit::Stream& s) { | ||
| unsigned char t; | ||
| s >> t; | ||
| type_ = static_cast<WipeElementType>(t); | ||
| s >> msg_; | ||
| size_t numURIs; | ||
| s >> numURIs; | ||
| for (size_t i = 0; i < numURIs; i++) { | ||
| uris_.insert(eckit::URI{s}); | ||
| } | ||
| } | ||
|
|
||
| void WipeElement::print(std::ostream& out) const { | ||
| // out << "Wipe(type=" << type_ << ",msg=" << msg_ << ",uris=["; | ||
| // std::string sep = ""; | ||
| // for (const auto& uri : uris_) { | ||
| // out << sep << uri; | ||
| // sep = ","; | ||
| // } | ||
| // out << "])"; | ||
|
Comment on lines
+40
to
+46
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. del |
||
| out << msg_ << std::endl; | ||
| if (type_ != WIPE_CATALOGUE_INFO && type_ != WIPE_STORE_INFO) { | ||
| if (uris_.size() > 0) { | ||
| for (const auto& uri : uris_) { | ||
| out << " " << uri.asString() << std::endl; | ||
| } | ||
| out << std::endl; | ||
| } | ||
| else { | ||
| out << " - NONE -" << std::endl; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| size_t encodeSizeString(const std::string& s) { | ||
| return (1 + 4 + s.size()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the 1 and the 4 for? |
||
| } | ||
|
|
||
| size_t WipeElement::encodeSize() const { | ||
| size_t aux = 0; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So... aux here is the estimated size of all the uris combined? Why is it called aux? |
||
| for (const auto& l : uris_) { | ||
| aux += 256; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 256? wot?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Surely this should add the length of the URI? Or something dependent on that. |
||
| } | ||
| return 1 + 4 + encodeSizeString(msg_) + 1 + 4 + aux; | ||
| } | ||
|
|
||
| void WipeElement::encode(eckit::Stream& s) const { | ||
| s << static_cast<unsigned char>(type_); | ||
| s << msg_; | ||
| s << uris_.size(); | ||
| for (const auto& uri : uris_) { | ||
| s << uri; | ||
| } | ||
| } | ||
|
|
||
| //---------------------------------------------------------------------------------------------------------------------- | ||
|
|
||
| } // namespace fdb5 | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this file should be deleted? It is a (older/newer) copy of src/fdb5/api/helpers/WipeIterator.cc