Skip to content

Commit

Permalink
Denoise Precondition failed logs
Browse files Browse the repository at this point in the history
Because aws-sdk-cpp transforms PreconditionFailed errors into
S3Error::Unkown we do string munging on the exception name to identify
it.

Also fixes some block version ref key conflicts
  • Loading branch information
IvoDD committed Dec 9, 2024
1 parent e870a13 commit 2a6b3cd
Show file tree
Hide file tree
Showing 6 changed files with 10 additions and 5 deletions.
4 changes: 2 additions & 2 deletions cpp/arcticdb/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,7 @@ if(${TEST})
util/test/test_id_transformation.cpp
util/test/test_key_utils.cpp
util/test/test_ranges_from_future.cpp
util/test/test_reliable_storage_lock.cpp
util/test/test_slab_allocator.cpp
util/test/test_storage_lock.cpp
util/test/test_string_pool.cpp
Expand All @@ -964,8 +965,7 @@ if(${TEST})
version/test/test_version_store.cpp
version/test/version_map_model.hpp
python/python_handlers.cpp
storage/test/common.hpp
util/test/test_reliable_storage_lock.cpp)
storage/test/common.hpp)

set(EXECUTABLE_PERMS OWNER_WRITE OWNER_READ OWNER_EXECUTE GROUP_READ GROUP_EXECUTE WORLD_READ WORLD_EXECUTE) # 755

Expand Down
2 changes: 1 addition & 1 deletion cpp/arcticdb/entity/key.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ enum class KeyType : int {
/*
* Used for a list based reliable storage lock
*/
ATOMIC_LOCK = 27,
ATOMIC_LOCK = 28,
UNDEFINED
};

Expand Down
4 changes: 4 additions & 0 deletions cpp/arcticdb/storage/s3/detail-inl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ namespace s3 {
error_message_suffix));
}

if (type == Aws::S3::S3Errors::UNKNOWN && err.GetExceptionName().find("Precondition") != std::string::npos) {
raise<ErrorCode::E_ATOMIC_OPERATION_FAILED>(fmt::format("Atomic operation failed: {}", error_message_suffix));
}

// We create a more detailed error explanation in case of NETWORK_CONNECTION errors to remedy #880.
if (type == Aws::S3::S3Errors::NETWORK_CONNECTION) {
error_message = fmt::format("Unexpected network error: {} "
Expand Down
2 changes: 1 addition & 1 deletion cpp/arcticdb/storage/test/in_memory_store.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ namespace arcticdb {
ARCTICDB_DEBUG(log::storage(), "Adding segment with key {}", key);
if (if_none_match) {
if (seg_by_ref_key_.find(key) != seg_by_ref_key_.end()) {
storage::raise<ErrorCode::E_UNEXPECTED_S3_ERROR>("Precondition failed. Object is already present.");
storage::raise<ErrorCode::E_ATOMIC_OPERATION_FAILED>("Precondition failed. Object is already present.");
}
}
seg_by_ref_key_[key] = std::make_unique<SegmentInMemory>(std::move(seg));
Expand Down
1 change: 1 addition & 0 deletions cpp/arcticdb/util/error_code.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ inline std::unordered_map<ErrorCategory, const char*> get_error_category_names()
ERROR_CODE(5011, E_UNEXPECTED_LMDB_ERROR) \
ERROR_CODE(5020, E_UNEXPECTED_S3_ERROR) \
ERROR_CODE(5021, E_S3_RETRYABLE) \
ERROR_CODE(5022, E_ATOMIC_OPERATION_FAILED) \
ERROR_CODE(5030, E_UNEXPECTED_AZURE_ERROR) \
ERROR_CODE(5050, E_MONGO_BULK_OP_NO_REPLY) \
ERROR_CODE(5051, E_UNEXPECTED_MONGO_ERROR) \
Expand Down
2 changes: 1 addition & 1 deletion cpp/arcticdb/util/reliable_storage_lock-inl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ std::optional<AcquiredLockId> ReliableStorageLock<ClockType>::try_take_next_id(c
// Either way it's safe to assume we have failed to acquire the lock in case of transient S3 error.
// If error persists we'll approprieately raise in the next attempt to LIST/GET the existing lock and propagate
// the transient error.
log::lock().warn("Failed to acquire lock (likely someone acquired it before us): {}", e.what());
log::lock().debug("Failed to acquire lock (likely someone acquired it before us): {}", e.what());
return std::nullopt;
}
// We clear old locks only after aquiring the lock to avoid duplicating the deletion work
Expand Down

0 comments on commit 2a6b3cd

Please sign in to comment.