From 85701e4514fd8fe9718370baaad82367e8a6d73c Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Mon, 24 Mar 2025 17:43:44 -0700 Subject: [PATCH] Fix handling of timeout conditions from storage backends --- src/storage/Backend.cc | 10 ++++---- src/storage/storage-async.bif | 6 ++++- testing/btest/Baseline/plugins.storage/output | 4 ++++ .../storage-plugin/src/StorageDummy.cc | 9 ++++++-- testing/btest/plugins/storage.zeek | 23 +++++++++++++++++-- 5 files changed, 43 insertions(+), 9 deletions(-) diff --git a/src/storage/Backend.cc b/src/storage/Backend.cc index 074ab3d083..3580ff4c72 100644 --- a/src/storage/Backend.cc +++ b/src/storage/Backend.cc @@ -28,10 +28,8 @@ ResultCallback::ResultCallback(zeek::detail::trigger::TriggerPtr trigger, const : trigger(std::move(trigger)), assoc(assoc) {} void ResultCallback::Timeout() { - static const auto& op_result_type = zeek::id::find_type("Storage::OperationResult"); - if ( ! IsSyncCallback() ) - trigger->Cache(assoc, OperationResult::MakeVal(ReturnCode::TIMEOUT).release()); + trigger->Cache(assoc, OperationResult::MakeVal(ReturnCode::TIMEOUT).get()); } void ResultCallback::Complete(OperationResult res) { @@ -121,7 +119,11 @@ OperationResult Backend::Erase(ResultCallback* cb, ValPtr key) { } void Backend::CompleteCallback(ResultCallback* cb, const OperationResult& data) const { - cb->Complete(data); + if ( data.code == ReturnCode::TIMEOUT ) + cb->Timeout(); + else + cb->Complete(data); + if ( ! cb->IsSyncCallback() ) { delete cb; } diff --git a/src/storage/storage-async.bif b/src/storage/storage-async.bif index 08e74e90e5..ff56cef0cd 100644 --- a/src/storage/storage-async.bif +++ b/src/storage/storage-async.bif @@ -55,7 +55,11 @@ static void handle_async_result(const IntrusivePtr& backend, ResultCall // it didn't report back IN_PROGRESS. // 2. The backend doesn't support async. This means we already blocked in order // to get here already. - cb->Complete(op_result); + // Also check for timeout conditions. + if ( op_result.code == ReturnCode::TIMEOUT ) + cb->Timeout(); + else + cb->Complete(op_result); delete cb; } else if ( run_state::reading_traces ) { diff --git a/testing/btest/Baseline/plugins.storage/output b/testing/btest/Baseline/plugins.storage/output index 46752d25d5..89c4f9ab07 100644 --- a/testing/btest/Baseline/plugins.storage/output +++ b/testing/btest/Baseline/plugins.storage/output @@ -12,3 +12,7 @@ results of trying to use closed handle: get: Storage::NOT_CONNECTED, put: Storag open result 2, [code=Storage::OPERATION_FAILED, error_str=Failed to open backend Storage::STORAGEDUMMY: open_fail was set to true, returning error, value=] close result on closed handle, [code=Storage::NOT_CONNECTED, error_str=Backend is closed, value=] + +open result 3, [code=Storage::SUCCESS, error_str=, value=] +put timed out +close result, [code=Storage::SUCCESS, error_str=, value=] diff --git a/testing/btest/plugins/storage-plugin/src/StorageDummy.cc b/testing/btest/plugins/storage-plugin/src/StorageDummy.cc index 5c8d0ae770..63739e3a5b 100644 --- a/testing/btest/plugins/storage-plugin/src/StorageDummy.cc +++ b/testing/btest/plugins/storage-plugin/src/StorageDummy.cc @@ -21,8 +21,8 @@ BackendPtr StorageDummy::Instantiate(std::string_view tag) { return make_intrusi * with a corresponding message. */ OperationResult StorageDummy::DoOpen(OpenResultCallback* cb, RecordValPtr options) { - RecordValPtr backend_options = options->GetField("dummy"); - bool open_fail = backend_options->GetField("open_fail")->Get(); + RecordValPtr dummy_options = options->GetField("dummy"); + bool open_fail = dummy_options->GetField("open_fail")->Get(); if ( open_fail ) return {ReturnCode::OPERATION_FAILED, "open_fail was set to true, returning error"}; @@ -44,6 +44,11 @@ OperationResult StorageDummy::DoClose(ResultCallback* cb) { */ OperationResult StorageDummy::DoPut(ResultCallback* cb, ValPtr key, ValPtr value, bool overwrite, double expiration_time) { + RecordValPtr dummy_options = backend_options->GetField("dummy"); + bool timeout_put = dummy_options->GetField("timeout_put")->Get(); + if ( timeout_put ) + return {ReturnCode::TIMEOUT}; + auto json_key = key->ToJSON()->ToStdString(); auto json_value = value->ToJSON()->ToStdString(); data[json_key] = json_value; diff --git a/testing/btest/plugins/storage.zeek b/testing/btest/plugins/storage.zeek index 8bf93f38c4..2089497d9f 100644 --- a/testing/btest/plugins/storage.zeek +++ b/testing/btest/plugins/storage.zeek @@ -8,10 +8,12 @@ # @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff output # @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff zeek-stderr +@load base/frameworks/storage/async @load base/frameworks/storage/sync type StorageDummyOpts : record { open_fail: bool; + timeout_put: bool; }; redef record Storage::BackendOptions += { @@ -20,7 +22,7 @@ redef record Storage::BackendOptions += { event zeek_init() { local opts : Storage::BackendOptions; - opts$dummy = [$open_fail = F]; + opts$dummy = [$open_fail = F, $timeout_put = F]; local key = "key1234"; local value = "value5678"; @@ -60,9 +62,26 @@ event zeek_init() { print ""; # Test failing to open the handle and test closing an invalid handle. - opts$dummy = [$open_fail = T]; + opts$dummy = [$open_fail = T, $timeout_put = F]; res = Storage::Sync::open_backend(Storage::STORAGEDUMMY, opts, string, string); print "open result 2", res; res = Storage::Sync::close_backend(res$value); print "close result on closed handle", res; + print ""; + + # Test timing out an async put request. + opts$dummy = [$open_fail = F, $timeout_put = T]; + res = Storage::Sync::open_backend(Storage::STORAGEDUMMY, opts, string, string); + print "open result 3", res; + b = res$value; + + when [b, key, value] ( local res2 = Storage::Async::put(b, [$key=key, $value=value]) ) { + local when_res = Storage::Sync::close_backend(b); + print "FAIL: should not happen: close result", when_res; + } + timeout 5sec { + print "put timed out"; + local to_res = Storage::Sync::close_backend(b); + print "close result", to_res; + } }