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

Issue #1017 Only warn if the "base" LMDB env is opened twice #1022

Merged
merged 1 commit into from
Oct 31, 2023
Merged

Issue #1017 Only warn if the "base" LMDB env is opened twice #1022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 13 additions & 21 deletions cpp/arcticdb/storage/lmdb/lmdb_storage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ LmdbStorage::LmdbStorage(const LibraryPath &library_path, OpenMode mode, const C

lib_dir_ = root_path / lib_path_str;

warn_if_lmdb_already_open(root_path, lib_path_str);
warn_if_lmdb_already_open();

if (!fs::exists(lib_dir_)) {
util::check_arg(mode > OpenMode::READ, "Missing dir {} for lib={}. mode={}",
Expand Down Expand Up @@ -326,27 +326,19 @@ LmdbStorage::LmdbStorage(const LibraryPath &library_path, OpenMode mode, const C
ARCTICDB_DEBUG(log::storage(), "Opened lmdb storage at {} with map size {}", lib_dir_.string(), format_bytes(mapsize));
}

void LmdbStorage::warn_if_lmdb_already_open(const fs::path &root_path, const std::string &lib_path_str) {
uint64_t& count_for_pid = ++times_path_opened[(root_path / lib_path_str).string()];
if (count_for_pid != 1) {
void LmdbStorage::warn_if_lmdb_already_open() {
uint64_t& count_for_pid = ++times_path_opened[lib_dir_.string()];
// Only warn for the "base" config library to avoid spamming users with more warnings if they decide to ignore it and continue
if (count_for_pid != 1 && lib_dir_.string().find(CONFIG_LIBRARY_NAME) != std::string::npos) {
std::filesystem::path user_facing_path = lib_dir_;
// Strip magic name from warning as it will confuse users
if (lib_dir_.string().find(CONFIG_LIBRARY_NAME) == std::string::npos) {
log::storage().warn(fmt::format(
"LMDB path at {} has already been opened in this process which is not supported by LMDB. "
"To continue safely, you should delete all Library objects over the LMDB path in this "
"process and then try again. This indicates a bug in ArcticDB. Please report at "
"https://github.com/man-group/ArcticDB. Current process ID=[{}]",
lib_dir_.string(), getpid()));
} else {
std::filesystem::path user_facing_path = lib_dir_;
user_facing_path.remove_filename();
log::storage().warn(fmt::format(
"LMDB path at {} has already been opened in this process which is not supported by LMDB. "
"You should only open a single Arctic instance over a given LMDB path. "
"To continue safely, you should delete this Arctic instance and any others over the LMDB path in this "
"process and then try again. Current process ID=[{}]",
user_facing_path.string(), getpid()));
}
user_facing_path.remove_filename();
log::storage().warn(fmt::format(
"LMDB path at {} has already been opened in this process which is not supported by LMDB. "
"You should only open a single Arctic instance over a given LMDB path. "
"To continue safely, you should delete this Arctic instance and any others over the LMDB path in this "
"process and then try again. Current process ID=[{}]",
user_facing_path.string(), getpid()));
}
}

Expand Down
2 changes: 1 addition & 1 deletion cpp/arcticdb/storage/lmdb/lmdb_storage.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class LmdbStorage final : public Storage {

std::string do_key_path(const VariantKey&) const final { return {}; };

void warn_if_lmdb_already_open(const fs::path &root_path, const std::string &lib_path_str);
void warn_if_lmdb_already_open();

// _internal methods assume the write mutex is already held
void do_write_internal(Composite<KeySegmentPair>&& kvs, ::lmdb::txn& txn);
Expand Down
7 changes: 7 additions & 0 deletions python/tests/integration/arcticdb/test_lmdb.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,3 +208,10 @@ def test_arctic_instances_across_same_lmdb_multiprocessing(tmpdir):
ac["test"].write("a", pd.DataFrame())
with mp.Pool(5) as p:
p.starmap(create_arctic_instance, [(tmpdir, i) for i in range(20)])


def test_lmdb_warnings_when_reopened(tmpdir):
ac = Arctic(f"lmdb://{tmpdir}")
lib = ac.create_library("test")
ac = Arctic(f"lmdb://{tmpdir}") # expect to warn
lib = ac["test"]
Loading