From 68f0fe9e8c74a82301584edac5a021228ea2fad5 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Thu, 23 Jan 2020 13:13:10 -0800 Subject: [PATCH 01/26] Automatic bro table->brokerstore insert operations We now have an &broker_store attribute which automatically sends inserts/deletes into a set/table to broker. This might work - I actually did not test if the data ends up in the broker store in the end. A limitation is that the table/set currently only can have a one-element type since Broker doesn't support the list type. --- src/Attr.cc | 26 +++++- src/Attr.h | 1 + src/Val.cc | 105 ++++++++++++++++++++++-- src/Val.h | 4 + src/parse.y | 6 +- src/scan.l | 1 + testing/btest/language/brokerstore.test | 24 ++++++ 7 files changed, 156 insertions(+), 11 deletions(-) create mode 100644 testing/btest/language/brokerstore.test diff --git a/src/Attr.cc b/src/Attr.cc index 72b984e924..6a56fcc275 100644 --- a/src/Attr.cc +++ b/src/Attr.cc @@ -14,7 +14,8 @@ const char* attr_name(attr_tag t) "&read_expire", "&write_expire", "&create_expire", "&raw_output", "&priority", "&group", "&log", "&error_handler", "&type_column", - "(&tracked)", "&deprecated", + "(&tracked)", "&on_change", "&broker_store", + "&deprecated", }; return attr_names[int(t)]; @@ -486,7 +487,7 @@ void Attributes::CheckAttr(Attr* a) { if ( type->Tag() != TYPE_TABLE ) { - Error("&on_change only applicable to tables"); + Error("&on_change only applicable to sets/tables"); break; } @@ -548,6 +549,27 @@ void Attributes::CheckAttr(Attr* a) } break; + case ATTR_BROKER_STORE: + { + if ( type->Tag() != TYPE_TABLE ) + { + Error("&broker_store only applicable to sets/tables"); + break; + } + + const Expr *broker_store = a->AttrExpr(); + if ( broker_store->Type()->Tag() != TYPE_OPAQUE || broker_store->Type()->AsOpaqueType()->Name() != "Broker::Store" ) + Error("&broker_store must take an opaque of Broker::Store"); + + + // Temporary since Broker does not support ListVals - and we cannot easily convert to set/vector + if ( type->AsTableType()->IndexTypes()->length() != 1 ) + { + Error("&broker_store only supports one-element set/table indexes"); + } + + } + case ATTR_TRACKED: // FIXME: Check here for global ID? break; diff --git a/src/Attr.h b/src/Attr.h index e9d75e7ae0..afb112f652 100644 --- a/src/Attr.h +++ b/src/Attr.h @@ -28,6 +28,7 @@ typedef enum { ATTR_TYPE_COLUMN, // for input framework ATTR_TRACKED, // hidden attribute, tracked by NotifierRegistry ATTR_ON_CHANGE, // for table change tracking + ATTR_BROKER_STORE, // for broker-store backed tables ATTR_DEPRECATED, #define NUM_ATTRS (int(ATTR_DEPRECATED) + 1) } attr_tag; diff --git a/src/Val.cc b/src/Val.cc index c5948d981c..921d952217 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -26,6 +26,7 @@ #include "IPAddr.h" #include "broker/Data.h" +#include "broker/Store.h" #include "threading/formatters/JSON.h" @@ -1415,6 +1416,12 @@ void TableVal::SetAttrs(Attributes* a) change_func = cf->AttrExpr(); change_func->Ref(); } + auto bs = attrs->FindAttr(ATTR_BROKER_STORE); + if ( bs ) + { + broker_store = bs->AttrExpr(); + broker_store->Ref(); + } } void TableVal::CheckExpireAttr(attr_tag at) @@ -1492,13 +1499,18 @@ int TableVal::Assign(Val* index, HashKey* k, Val* new_val) Modified(); - if ( change_func ) + if ( change_func || broker_store ) { auto change_index = index->Ref(); if ( ! change_index ) RecoverIndex(&k_copy); - Val *v = old_entry_val ? old_entry_val->Value() : new_val; - CallChangeFunc(change_index, v, old_entry_val ? element_changed : element_new); + if ( broker_store ) + SendToStore(change_index, new_val, old_entry_val ? element_changed : element_new); + if ( change_func ) + { + Val *v = old_entry_val ? old_entry_val->Value() : new_val; + CallChangeFunc(change_index, v, old_entry_val ? element_changed : element_new); + } Unref(change_index); } @@ -1971,9 +1983,7 @@ void TableVal::CallChangeFunc(const Val* index, Val* old_value, OnChangeType tpe Val* thefunc = change_func->Eval(0); if ( ! thefunc ) - { return; - } if ( thefunc->Type()->Tag() != TYPE_FUNC ) { @@ -2013,6 +2023,82 @@ void TableVal::CallChangeFunc(const Val* index, Val* old_value, OnChangeType tpe } } +void TableVal::SendToStore(const Val* index, const Val* new_value, OnChangeType tpe) + { + if ( ! broker_store || ! index ) + return; + + try + { + Val* thestore = broker_store->Eval(0); + + if ( ! thestore ) + return; + + if ( thestore->Type()->Tag() != TYPE_OPAQUE || broker_store->Type()->AsOpaqueType()->Name() != "Broker::Store" ) + { + thestore->Error("not a Broker::Store"); + Unref(thestore); + return; + } + + auto handle = static_cast(thestore); + if ( index->AsListVal()->Length() != 1 ) + { + builtin_error("table with complex index not supported for &broker_store"); + return; + } + + const auto index_val = index->AsListVal()->Index(0); + auto key_val = new StringVal("test"); + auto broker_key = bro_broker::val_to_data(key_val); + auto broker_index = bro_broker::val_to_data(index_val); + Unref(key_val); + + if ( ! broker_key ) + { + builtin_error("invalid Broker data conversion for &broker_store_key"); + return; + } + if ( ! broker_index ) + { + builtin_error("invalid Broker data conversation for table index"); + return; + } + + switch ( tpe ) + { + case element_new: + case element_changed: + if ( table_type->IsSet() ) + handle->store.insert_into(std::move(*broker_key), std::move(*broker_index)); + else + { + if ( ! new_value ) + { + builtin_error("did not receive new value for broker-store send operation"); + return; + } + auto broker_val = bro_broker::val_to_data(new_value); + if ( ! broker_val ) + { + builtin_error("invalid Broker data conversation for table value"); + return; + } + handle->store.insert_into(std::move(*broker_key), std::move(*broker_index), std::move(*broker_val)); + } + break; + case element_removed: + handle->store.remove_from(std::move(*broker_key), std::move(*broker_index)); + break; + } + } + catch ( InterpreterException& e ) + { + builtin_error("The previous error was encountered while trying to resolve the &broker_store attribute of the set/table. Potentially the Broker::Store has not been initialized before being used."); + } + } + Val* TableVal::Delete(const Val* index) { HashKey* k = ComputeHash(index); @@ -2027,6 +2113,8 @@ Val* TableVal::Delete(const Val* index) Modified(); + if ( broker_store ) + SendToStore(index, nullptr, element_removed); if ( change_func ) CallChangeFunc(index, va, element_removed); @@ -2050,10 +2138,13 @@ Val* TableVal::Delete(const HashKey* k) Modified(); - if ( change_func ) + if ( change_func || broker_store ) { auto index = table_hash->RecoverVals(k); - CallChangeFunc(index, va->Ref(), element_removed); + if ( broker_store ) + SendToStore(index, nullptr, element_removed); + if ( change_func ) + CallChangeFunc(index, va->Ref(), element_removed); Unref(index); } diff --git a/src/Val.h b/src/Val.h index 9124df2f74..bd1a051d6a 100644 --- a/src/Val.h +++ b/src/Val.h @@ -928,6 +928,9 @@ protected: // Calls &change_func. Does not take ownership of values. (Refs if needed). void CallChangeFunc(const Val* index, Val* old_value, OnChangeType tpe); + // Sends data on to backing Broker Store + void SendToStore(const Val* index, const Val* new_value, OnChangeType tpe); + Val* DoClone(CloneState* state) override; TableType* table_type; @@ -940,6 +943,7 @@ protected: PrefixTable* subnets; Val* def_val; Expr* change_func = nullptr; + Expr* broker_store = nullptr; }; class RecordVal : public Val, public notifier::Modifiable { diff --git a/src/parse.y b/src/parse.y index ff65c4b184..58adc2a21d 100644 --- a/src/parse.y +++ b/src/parse.y @@ -5,7 +5,7 @@ // Switching parser table type fixes ambiguity problems. %define lr.type ielr -%expect 111 +%expect 117 %token TOK_ADD TOK_ADD_TO TOK_ADDR TOK_ANY %token TOK_ATENDIF TOK_ATELSE TOK_ATIF TOK_ATIFDEF TOK_ATIFNDEF @@ -24,7 +24,7 @@ %token TOK_ATTR_ADD_FUNC TOK_ATTR_DEFAULT TOK_ATTR_OPTIONAL TOK_ATTR_REDEF %token TOK_ATTR_DEL_FUNC TOK_ATTR_EXPIRE_FUNC %token TOK_ATTR_EXPIRE_CREATE TOK_ATTR_EXPIRE_READ TOK_ATTR_EXPIRE_WRITE -%token TOK_ATTR_RAW_OUTPUT TOK_ATTR_ON_CHANGE +%token TOK_ATTR_RAW_OUTPUT TOK_ATTR_ON_CHANGE TOK_ATTR_BROKER_STORE %token TOK_ATTR_PRIORITY TOK_ATTR_LOG TOK_ATTR_ERROR_HANDLER %token TOK_ATTR_TYPE_COLUMN TOK_ATTR_DEPRECATED @@ -1329,6 +1329,8 @@ attr: { $$ = new Attr(ATTR_DEL_FUNC, $3); } | TOK_ATTR_ON_CHANGE '=' expr { $$ = new Attr(ATTR_ON_CHANGE, $3); } + | TOK_ATTR_BROKER_STORE '=' expr + { $$ = new Attr(ATTR_BROKER_STORE, $3); } | TOK_ATTR_EXPIRE_FUNC '=' expr { $$ = new Attr(ATTR_EXPIRE_FUNC, $3); } | TOK_ATTR_EXPIRE_CREATE '=' expr diff --git a/src/scan.l b/src/scan.l index d8792a1470..704c0452c6 100644 --- a/src/scan.l +++ b/src/scan.l @@ -299,6 +299,7 @@ when return TOK_WHEN; &redef return TOK_ATTR_REDEF; &write_expire return TOK_ATTR_EXPIRE_WRITE; &on_change return TOK_ATTR_ON_CHANGE; +&broker_store return TOK_ATTR_BROKER_STORE; @deprecated.* { auto num_files = file_stack.length(); diff --git a/testing/btest/language/brokerstore.test b/testing/btest/language/brokerstore.test new file mode 100644 index 0000000000..021a0d7463 --- /dev/null +++ b/testing/btest/language/brokerstore.test @@ -0,0 +1,24 @@ +# @TEST-EXEC: zeek %INPUT >output +# @TEST-EXEC: btest-diff output + +module TestModule; + +global tablestore: opaque of Broker::Store; +global setstore: opaque of Broker::Store; + +global t: table[string] of count &broker_store=tablestore; +global s: set[string] &broker_store=setstore; + +event zeek_init() + { + tablestore = Broker::create_master("table"); + setstore = Broker::create_master("set"); + print "inserting"; + t["a"] = 5; + add s["hi"]; + print "changing"; + t["a"] = 2; + print "deleting"; + delete t["a"]; + delete s["hi"]; + } From 8db83a5ed276dfb1f1af78eef9f60d55e0190c5b Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Thu, 28 May 2020 12:26:58 -0700 Subject: [PATCH 02/26] Make &broker_store take argument of type string. It turns out that directly passing a Broker::Store is not really a bright idea. Because - if we do that - we have to later try to intercept when the master/clone is generated to figure out what the actual name of the backing store is. Turns out that it is much easier to just use the name directly - and check if a store with that name exists when we want to insert something. I might want to reconsider this in the future in any case. At the moment this approach just stores one table into an entire store. In theory, one store should be able to handle several tables, but... that's more complex. So let's start with this for now. --- src/Attr.cc | 9 +++-- src/Val.cc | 38 +++++++++--------- src/Val.h | 2 +- src/broker/Manager.cc | 52 ++++++++++++++++++++++++- src/broker/Manager.h | 6 +++ src/broker/Store.h | 1 + src/scan.l | 1 + testing/btest/language/brokerstore.test | 6 ++- 8 files changed, 88 insertions(+), 27 deletions(-) diff --git a/src/Attr.cc b/src/Attr.cc index 32c318a825..6a736629ac 100644 --- a/src/Attr.cc +++ b/src/Attr.cc @@ -558,10 +558,11 @@ void Attributes::CheckAttr(Attr* a) break; } - const Expr *broker_store = a->AttrExpr(); - if ( broker_store->Type()->Tag() != TYPE_OPAQUE || broker_store->Type()->AsOpaqueType()->Name() != "Broker::Store" ) - Error("&broker_store must take an opaque of Broker::Store"); - + if ( a->AttrExpr()->Type()->Tag() != TYPE_STRING ) + { + Error("&broker_store must take a string argument"); + break; + } // Temporary since Broker does not support ListVals - and we cannot easily convert to set/vector if ( type->AsTableType()->IndexTypes()->length() != 1 ) diff --git a/src/Val.cc b/src/Val.cc index e023ce69f5..c38f185a8c 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -38,6 +38,7 @@ #include "broker/Data.h" #include "broker/Store.h" +#include "broker/Manager.h" #include "threading/formatters/JSON.h" @@ -1471,8 +1472,14 @@ void TableVal::SetAttrs(IntrusivePtr a) change_func = {NewRef{}, cf->AttrExpr()}; auto bs = attrs->FindAttr(ATTR_BROKER_STORE); - if ( bs ) - broker_store = {NewRef{}, bs->AttrExpr()}; + if ( bs && broker_store.empty() ) // this does not mesh well with being updated several times + { + IntrusivePtr c = bs->AttrExpr()->Eval(nullptr); + assert(c); + assert(c->Type()->Tag() == TYPE_STRING); + broker_store = c->AsStringVal()->AsString()->CheckString(); + broker_mgr->AddForwardedStore(broker_store, {NewRef{}, this}); + } } void TableVal::CheckExpireAttr(attr_tag at) @@ -1552,16 +1559,16 @@ bool TableVal::Assign(Val* index, HashKey* k, IntrusivePtr new_val) Modified(); - if ( change_func || broker_store ) + if ( change_func || ( ! broker_store.empty() ) ) { auto change_index = index ? IntrusivePtr{NewRef{}, index} : RecoverIndex(&k_copy); - if ( broker_store ) + if ( ! broker_store.empty() ) SendToStore(change_index.get(), new_val.get(), old_entry_val ? ELEMENT_CHANGED : ELEMENT_NEW); if ( change_func ) { - Val* v = old_entry_val ? old_entry_val->Value() : new_val.get(); - CallChangeFunc(change_index.get(), v, old_entry_val ? ELEMENT_CHANGED : ELEMENT_NEW); + Val* v = old_entry_val ? old_entry_val->Value() : new_val.get(); + CallChangeFunc(change_index.get(), v, old_entry_val ? ELEMENT_CHANGED : ELEMENT_NEW); } } @@ -2062,23 +2069,16 @@ void TableVal::CallChangeFunc(const Val* index, Val* old_value, OnChangeType tpe void TableVal::SendToStore(const Val* index, const Val* new_value, OnChangeType tpe) { - if ( ! broker_store || ! index ) + if ( broker_store.empty() || ! index ) return; try { - auto thestore = broker_store->Eval(0); + auto handle = broker_mgr->LookupStore(broker_store); - if ( ! thestore ) + if ( ! handle ) return; - if ( thestore->Type()->Tag() != TYPE_OPAQUE || broker_store->Type()->AsOpaqueType()->Name() != "Broker::Store" ) - { - thestore->Error("not a Broker::Store"); - return; - } - - auto handle = static_cast(thestore.get()); if ( index->AsListVal()->Length() != 1 ) { builtin_error("table with complex index not supported for &broker_store"); @@ -2149,7 +2149,7 @@ IntrusivePtr TableVal::Delete(const Val* index) Modified(); - if ( broker_store ) + if ( ! broker_store.empty() ) SendToStore(index, nullptr, ELEMENT_REMOVED); if ( change_func ) CallChangeFunc(index, va.get(), ELEMENT_REMOVED); @@ -2174,10 +2174,10 @@ IntrusivePtr TableVal::Delete(const HashKey* k) Modified(); - if ( ( change_func && va ) || broker_store ) + if ( ( change_func && va ) || ( ! broker_store.empty() ) ) { auto index = table_hash->RecoverVals(k); - if ( broker_store ) + if ( ! broker_store.empty() ) SendToStore(index.get(), nullptr, ELEMENT_REMOVED); if ( change_func && va ) CallChangeFunc(index.get(), va.get(), ELEMENT_REMOVED); diff --git a/src/Val.h b/src/Val.h index 4a074cdfa3..fdaa917565 100644 --- a/src/Val.h +++ b/src/Val.h @@ -895,7 +895,7 @@ protected: PrefixTable* subnets; IntrusivePtr def_val; IntrusivePtr change_func; - IntrusivePtr broker_store; + std::string broker_store; // prevent recursion of change functions bool in_change_func = false; diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index d54b061580..d7ce3a3054 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -212,6 +212,8 @@ void Manager::InitPostScript() reporter->FatalError("Failed to register broker subscriber with iosource_mgr"); if ( ! iosource_mgr->RegisterFd(bstate->status_subscriber.fd(), this) ) reporter->FatalError("Failed to register broker status subscriber with iosource_mgr"); + + bstate->subscriber.add_topic(broker::topics::store_events, true); } void Manager::Terminate() @@ -905,6 +907,20 @@ void Manager::Process() auto& topic = broker::get_topic(message); auto& msg = broker::get_data(message); + if ( topic == broker::topics::store_events ) + { + if (auto insert = broker::store_event::insert::make(msg)) + { + reporter->Warning("It is an insert!"); + reporter->Warning("Key/Data (endpoint): %s/%s (%s)", to_string(insert.key()).c_str(), to_string(insert.value()).c_str(), to_string(insert.publisher()).c_str()); + } + else + { + reporter->Warning("Unhandled event type"); + } + continue; + } + try { DispatchMessage(topic, std::move(msg)); @@ -1449,6 +1465,7 @@ StoreHandleVal* Manager::MakeMaster(const string& name, broker::backend type, data_stores.emplace(name, handle); iosource_mgr->RegisterFd(handle->proxy.mailbox().descriptor(), this); + CheckForwarding(name); if ( bstate->endpoint.use_real_time() ) return handle; @@ -1486,7 +1503,7 @@ StoreHandleVal* Manager::MakeClone(const string& name, double resync_interval, data_stores.emplace(name, handle); iosource_mgr->RegisterFd(handle->proxy.mailbox().descriptor(), this); - + CheckForwarding(name); return handle; } @@ -1504,6 +1521,9 @@ bool Manager::CloseStore(const string& name) if ( s == data_stores.end() ) return false; + auto pubid = s->second->store.frontend_id(); + forwarded_ids.erase(pubid); + iosource_mgr->UnregisterFd(s->second->proxy.mailbox().descriptor(), this); for ( auto i = pending_queries.begin(); i != pending_queries.end(); ) @@ -1546,4 +1566,34 @@ const Stats& Manager::GetStatistics() return statistics; } +bool Manager::AddForwardedStore(const std::string& name, IntrusivePtr table) + { + if ( forwarded_stores.find(name) != forwarded_stores.end() ) + { + reporter->Error("same &broker_store %s specified for two different variables", name.c_str()); + return false; + } + + DBG_LOG(DBG_BROKER, "Adding table forward for data store %s", name.c_str()); + forwarded_stores.emplace(name, table); + + CheckForwarding(name); + return true; + } + +void Manager::CheckForwarding(const std::string &name) + { + auto handle = LookupStore(name); + if ( ! handle ) + return; + + if ( forwarded_stores.find(name) == forwarded_stores.end() ) + return; + + auto pubid = handle->store.frontend_id(); + + DBG_LOG(DBG_BROKER, "Resolved publishder %s for table forward for data store %s", to_string(pubid).c_str(), name.c_str()); + forwarded_ids.emplace(pubid, forwarded_stores.at(name)); + } + } // namespace bro_broker diff --git a/src/broker/Manager.h b/src/broker/Manager.h index bbd38f88be..2746b89142 100644 --- a/src/broker/Manager.h +++ b/src/broker/Manager.h @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -294,6 +295,8 @@ public: */ StoreHandleVal* LookupStore(const std::string& name); + bool AddForwardedStore(const std::string& name, IntrusivePtr table); + /** * Close and unregister a data store. Any existing references to the * store handle will not be able to be used for any data store operations. @@ -347,6 +350,7 @@ private: void ProcessError(broker::error err); void ProcessStoreResponse(StoreHandleVal*, broker::store::response response); void FlushPendingQueries(); + void CheckForwarding(const std::string& name); void Error(const char* format, ...) __attribute__((format (printf, 2, 3))); @@ -381,6 +385,8 @@ private: std::string default_log_topic_prefix; std::shared_ptr bstate; std::unordered_map data_stores; + std::unordered_map> forwarded_stores; + std::unordered_map> forwarded_ids; std::unordered_map pending_queries; std::vector forwarded_prefixes; diff --git a/src/broker/Store.h b/src/broker/Store.h index 09799c8c68..4bf6dee717 100644 --- a/src/broker/Store.h +++ b/src/broker/Store.h @@ -6,6 +6,7 @@ #include "Trigger.h" #include +#include #include #include diff --git a/src/scan.l b/src/scan.l index 0fd1a65085..6f598cfe08 100644 --- a/src/scan.l +++ b/src/scan.l @@ -310,6 +310,7 @@ when return TOK_WHEN; &redef return TOK_ATTR_REDEF; &write_expire return TOK_ATTR_EXPIRE_WRITE; &on_change return TOK_ATTR_ON_CHANGE; +&broker_store return TOK_ATTR_BROKER_STORE; @deprecated.* { auto num_files = file_stack.length(); diff --git a/testing/btest/language/brokerstore.test b/testing/btest/language/brokerstore.test index 021a0d7463..cb1e207b36 100644 --- a/testing/btest/language/brokerstore.test +++ b/testing/btest/language/brokerstore.test @@ -1,13 +1,15 @@ # @TEST-EXEC: zeek %INPUT >output # @TEST-EXEC: btest-diff output +redef exit_only_after_terminate = T; + module TestModule; global tablestore: opaque of Broker::Store; global setstore: opaque of Broker::Store; -global t: table[string] of count &broker_store=tablestore; -global s: set[string] &broker_store=setstore; +global t: table[string] of count &broker_store="table"; +global s: set[string] &broker_store="set"; event zeek_init() { From 558e89b3ba6c9d1cc4ac12426f8fe226ff38972b Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Fri, 29 May 2020 14:32:16 -0700 Subject: [PATCH 03/26] Broker Store updates: get a bit more infrastructure in place. This compiles, but besides giving debug messages (and partially performing inserts/updates) it is not really helpful and definitely WIP. This also shows that I might have to re-think the approach that we will take here. So far, we actually insert tables as tables into Brokerstores. This opens up the potential to just have several tables synchronized via a single brokerstore. However, it turns out, that the current store_event API sends the complete table with each update. Which is problematic for obvious reasons - and not really sustainable. --- src/Val.cc | 23 +++- src/broker/Manager.cc | 143 +++++++++++++++++++++--- src/broker/Manager.h | 2 +- src/broker/Store.h | 5 +- testing/btest/language/brokerstore.test | 17 ++- 5 files changed, 164 insertions(+), 26 deletions(-) diff --git a/src/Val.cc b/src/Val.cc index c38f185a8c..a88474d994 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -2079,14 +2079,27 @@ void TableVal::SendToStore(const Val* index, const Val* new_value, OnChangeType if ( ! handle ) return; - if ( index->AsListVal()->Length() != 1 ) + // we wither get passed the raw index_val - or a ListVal with exactly one element. + // Since broker does not support ListVals, we have to unoll this in the second case. + const Val* index_val; + if ( index->Type()->Tag() == TYPE_LIST ) { - builtin_error("table with complex index not supported for &broker_store"); - return; + if ( index->AsListVal()->Length() != 1 ) + { + builtin_error("table with complex index not supported for &broker_store"); + return; + } + + index_val = index->AsListVal()->Index(0); + } + else + { + index_val = index; } - const auto index_val = index->AsListVal()->Index(0); - auto key_val = new StringVal("test"); + // FIXME: at the moment this is hardcoded to the name of the broker store. I needed something to be able to tell + // me which store a change came from - and this still seems to be missing from the store_events. (Or I am blind). + auto key_val = new StringVal(broker_store); auto broker_key = bro_broker::val_to_data(key_val); auto broker_index = bro_broker::val_to_data(index_val); Unref(key_val); diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index d7ce3a3054..d99f47423c 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -909,15 +909,7 @@ void Manager::Process() if ( topic == broker::topics::store_events ) { - if (auto insert = broker::store_event::insert::make(msg)) - { - reporter->Warning("It is an insert!"); - reporter->Warning("Key/Data (endpoint): %s/%s (%s)", to_string(insert.key()).c_str(), to_string(insert.value()).c_str(), to_string(insert.publisher()).c_str()); - } - else - { - reporter->Warning("Unhandled event type"); - } + ProcessStoreEvent(topic, std::move(msg)); continue; } @@ -956,6 +948,132 @@ void Manager::Process() } } +void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) + { + // for all of the following we currently cheat. The key always is the name of the store. This will change + // in the future. + if ( auto insert = broker::store_event::insert::make(msg) ) + { + auto remstore = to_string(insert.key()); + auto storehandle = broker_mgr->LookupStore(remstore); + if ( ! storehandle ) + return; + + auto table = storehandle->forward_to; + if ( ! table ) + return; + + auto data = insert.value(); + + reporter->Warning("Insert Data: %s (%s)", to_string(insert.value()).c_str(), insert.value().get_type_name()); + if ( table->Type()->IsSet() && data.get_type() != broker::data::type::set ) + { + reporter->Error("ProcessStoreEvent Insert got %s when expecting set", data.get_type_name()); + return; + } + else if ( ! table->Type()->IsSet() && data.get_type() != broker::data::type::table ) + { + reporter->Error("ProcessStoreEvent Insert got %s when expecting table", data.get_type_name()); + return; + } + + // We sent this message. Ignore it. + //if ( insert.publisher() == storehandle->store_pid ) + // continue; + + // We currently use data_to_val here. It creates a bit of overhead, but makes the code easier. We could change + // this to manual unrolling in the future. + auto val = data_to_val(std::move(data), table->Type()); + // So we are just doing it manually - at least for now. + auto temptable = val->AsTable(); + auto temptableval = val->AsTableVal(); + HashKey* k; + TableEntryVal* entry; + auto c = temptable->InitForIteration(); + while ( (entry = temptable->NextEntry(k, c))) + { + auto lv = temptableval->RecoverIndex(k); + delete k; + Val* entry_key = lv->Length() == 1 ? lv->Index(0) : lv.get(); + + // note - at the moment this code creates a loop + // Fixme: expiry! + if ( temptableval->Type()->IsSet() ) + table->Assign(entry_key, nullptr); + else + table->Assign(entry_key, entry->Value()); + } + } + else if ( auto update = broker::store_event::update::make(msg) ) + { + auto remstore = to_string(update.key()); + auto storehandle = broker_mgr->LookupStore(remstore); + if ( ! storehandle ) + return; + + auto table = storehandle->forward_to; + if ( ! table ) + return; + + auto data = update.new_value(); + + reporter->Warning("Update Data: %s->%s (%s)", to_string(update.old_value()).c_str(), to_string(update.new_value()).c_str(), update.new_value().get_type_name()); + if ( table->Type()->IsSet() && data.get_type() != broker::data::type::set ) + { + reporter->Error("ProcessStoreEvent Update got %s when expecting set", data.get_type_name()); + return; + } + else if ( ! table->Type()->IsSet() && data.get_type() != broker::data::type::table ) + { + reporter->Error("ProcessStoreEvent Update got %s when expecting table", data.get_type_name()); + return; + } + + // We sent this message. Ignore it. + if ( update.publisher() == storehandle->store_pid ) + return; + + // We currently use data_to_val here. It creates a bit of overhead, but makes the code easier. We could change + // this to manual unrolling in the future. + auto val = data_to_val(std::move(data), table->Type()); + // So we are just doing it manually - at least for now. + auto temptable = val->AsTable(); + auto temptableval = val->AsTableVal(); + HashKey* k; + TableEntryVal* entry; + auto c = temptable->InitForIteration(); + while ( (entry = temptable->NextEntry(k, c))) + { + auto lv = temptableval->RecoverIndex(k); + delete k; + Val* entry_key = lv->Length() == 1 ? lv->Index(0) : lv.get(); + + // note - at the moment this code creates a loop + if ( temptableval->Type()->IsSet() ) + table->Assign(entry_key, nullptr); + else + table->Assign(entry_key, entry->Value()); + } + } + else if ( auto erase = broker::store_event::erase::make(msg) ) + { + auto remstore = to_string(erase.key()); + auto storehandle = broker_mgr->LookupStore(remstore); + if ( ! storehandle ) + return; + + auto table = storehandle->forward_to; + if ( ! table ) + return; + + // I'm actually not sure we can get an erase - since we will never delete keys... + reporter->Warning("Erase for key %s", remstore.c_str()); + } + else + { + reporter->Warning("Unhandled event type"); + } + } void Manager::ProcessEvent(const broker::topic& topic, broker::zeek::Event ev) { @@ -1522,7 +1640,6 @@ bool Manager::CloseStore(const string& name) return false; auto pubid = s->second->store.frontend_id(); - forwarded_ids.erase(pubid); iosource_mgr->UnregisterFd(s->second->proxy.mailbox().descriptor(), this); @@ -1590,10 +1707,8 @@ void Manager::CheckForwarding(const std::string &name) if ( forwarded_stores.find(name) == forwarded_stores.end() ) return; - auto pubid = handle->store.frontend_id(); - - DBG_LOG(DBG_BROKER, "Resolved publishder %s for table forward for data store %s", to_string(pubid).c_str(), name.c_str()); - forwarded_ids.emplace(pubid, forwarded_stores.at(name)); + handle->forward_to = forwarded_stores.at(name); + DBG_LOG(DBG_BROKER, "Resolved table forward for data store %s", name.c_str()); } } // namespace bro_broker diff --git a/src/broker/Manager.h b/src/broker/Manager.h index 2746b89142..dd04093945 100644 --- a/src/broker/Manager.h +++ b/src/broker/Manager.h @@ -342,6 +342,7 @@ public: private: void DispatchMessage(const broker::topic& topic, broker::data msg); + void ProcessStoreEvent(const broker::topic& topic, broker::data msg); void ProcessEvent(const broker::topic& topic, broker::zeek::Event ev); bool ProcessLogCreate(broker::zeek::LogCreate lc); bool ProcessLogWrite(broker::zeek::LogWrite lw); @@ -386,7 +387,6 @@ private: std::shared_ptr bstate; std::unordered_map data_stores; std::unordered_map> forwarded_stores; - std::unordered_map> forwarded_ids; std::unordered_map pending_queries; std::vector forwarded_prefixes; diff --git a/src/broker/Store.h b/src/broker/Store.h index 4bf6dee717..2f1462939e 100644 --- a/src/broker/Store.h +++ b/src/broker/Store.h @@ -95,13 +95,16 @@ private: class StoreHandleVal : public OpaqueVal { public: StoreHandleVal(broker::store s) - : OpaqueVal(bro_broker::opaque_of_store_handle), store{s}, proxy{store} + : OpaqueVal(bro_broker::opaque_of_store_handle), store{s}, proxy{store}, store_pid{store.frontend_id()} { } void ValDescribe(ODesc* d) const override; broker::store store; broker::store::proxy proxy; + broker::publisher_id store_pid; + // Zeek table that events are forwarded to. + IntrusivePtr forward_to; protected: StoreHandleVal() = default; diff --git a/testing/btest/language/brokerstore.test b/testing/btest/language/brokerstore.test index cb1e207b36..b983e83851 100644 --- a/testing/btest/language/brokerstore.test +++ b/testing/btest/language/brokerstore.test @@ -6,21 +6,28 @@ redef exit_only_after_terminate = T; module TestModule; global tablestore: opaque of Broker::Store; +#global tabletwostore: opaque of Broker::Store; global setstore: opaque of Broker::Store; global t: table[string] of count &broker_store="table"; +#global ct: table[string, string] of count &broker_store="table2"; global s: set[string] &broker_store="set"; event zeek_init() { tablestore = Broker::create_master("table"); + #tabletwostore = Broker::create_master("table2"); setstore = Broker::create_master("set"); print "inserting"; t["a"] = 5; - add s["hi"]; - print "changing"; - t["a"] = 2; - print "deleting"; delete t["a"]; - delete s["hi"]; + #add s["hi"]; + #print "changing"; + t["a"] = 2; + t["b"] = 3; + t["c"] = 4; + t["whatever"] = 5; + #print "deleting"; + #delete t["a"]; + #delete s["hi"]; } From ebb106c9b8ba26c45190e64f06c06781501c4105 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Fri, 29 May 2020 17:56:11 -0700 Subject: [PATCH 04/26] Store updates: downlevel and store table elements as keys. In this commit we change our approach and just store table keys as broker keys and table values as broker indexes. This means we only can have a single table in a broker store. This seems to work perfectly - and leads to vastly less complex code. Code should work, but is actually pretty untested. Might break with certain types. Does not yet handle item expiration. Also - this has the tiny issue that all remote operations currently lead to loops (the operation will immediately be sent to the broker-store again), which is not quite optimal. --- aux/broker | 2 +- src/Val.cc | 16 +-- src/broker/Manager.cc | 157 +++++++++++------------- testing/btest/language/brokerstore.test | 1 + 4 files changed, 79 insertions(+), 97 deletions(-) diff --git a/aux/broker b/aux/broker index 6974924007..60f3c7ffb2 160000 --- a/aux/broker +++ b/aux/broker @@ -1 +1 @@ -Subproject commit 6974924007765f70d95e5cf123b6256048ae3af7 +Subproject commit 60f3c7ffb2a2fb77c196f3066c4bc4c2577eb895 diff --git a/src/Val.cc b/src/Val.cc index a88474d994..da0271424b 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -2097,18 +2097,10 @@ void TableVal::SendToStore(const Val* index, const Val* new_value, OnChangeType index_val = index; } - // FIXME: at the moment this is hardcoded to the name of the broker store. I needed something to be able to tell + // FIXME: switch back to just storing tables directly in the broker store? // me which store a change came from - and this still seems to be missing from the store_events. (Or I am blind). - auto key_val = new StringVal(broker_store); - auto broker_key = bro_broker::val_to_data(key_val); auto broker_index = bro_broker::val_to_data(index_val); - Unref(key_val); - if ( ! broker_key ) - { - builtin_error("invalid Broker data conversion for &broker_store_key"); - return; - } if ( ! broker_index ) { builtin_error("invalid Broker data conversation for table index"); @@ -2120,7 +2112,7 @@ void TableVal::SendToStore(const Val* index, const Val* new_value, OnChangeType case ELEMENT_NEW: case ELEMENT_CHANGED: if ( table_type->IsSet() ) - handle->store.insert_into(std::move(*broker_key), std::move(*broker_index)); + handle->store.put(std::move(*broker_index), broker::data()); else { if ( ! new_value ) @@ -2134,11 +2126,11 @@ void TableVal::SendToStore(const Val* index, const Val* new_value, OnChangeType builtin_error("invalid Broker data conversation for table value"); return; } - handle->store.insert_into(std::move(*broker_key), std::move(*broker_index), std::move(*broker_val)); + handle->store.put(std::move(*broker_index), std::move(*broker_val)); } break; case ELEMENT_REMOVED: - handle->store.remove_from(std::move(*broker_key), std::move(*broker_index)); + handle->store.erase(std::move(*broker_index)); break; } } diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index d99f47423c..49ce79e2b6 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -907,7 +907,7 @@ void Manager::Process() auto& topic = broker::get_topic(message); auto& msg = broker::get_data(message); - if ( topic == broker::topics::store_events ) + if ( broker::topics::store_events.prefix_of(topic) ) { ProcessStoreEvent(topic, std::move(msg)); continue; @@ -950,128 +950,117 @@ void Manager::Process() void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) { - // for all of the following we currently cheat. The key always is the name of the store. This will change - // in the future. + auto topic_parts = broker::topic::split(topic); + const auto& store_name = topic_parts.back(); + + auto storehandle = broker_mgr->LookupStore(store_name); + if ( ! storehandle ) + return; + + auto table = storehandle->forward_to; + if ( ! table ) + return; + if ( auto insert = broker::store_event::insert::make(msg) ) { - auto remstore = to_string(insert.key()); - auto storehandle = broker_mgr->LookupStore(remstore); - if ( ! storehandle ) - return; - - auto table = storehandle->forward_to; - if ( ! table ) - return; - + auto key = insert.key(); auto data = insert.value(); - reporter->Warning("Insert Data: %s (%s)", to_string(insert.value()).c_str(), insert.value().get_type_name()); - if ( table->Type()->IsSet() && data.get_type() != broker::data::type::set ) + DBG_LOG(DBG_BROKER, "Insert Data: %s:%s (%s:%s)", to_string(insert.key()).c_str(), to_string(insert.value()).c_str(), insert.key().get_type_name(), insert.value().get_type_name()); + if ( table->Type()->IsSet() && data.get_type() != broker::data::type::none ) { reporter->Error("ProcessStoreEvent Insert got %s when expecting set", data.get_type_name()); return; } - else if ( ! table->Type()->IsSet() && data.get_type() != broker::data::type::table ) + + // We sent this message. Ignore it. + if ( insert.publisher() == storehandle->store_pid ) + return; + + // FIXME: expiry! + assert( table->Type()->AsTableType()->IndexTypes()->length() == 1 ); + auto zeek_key = data_to_val(std::move(key), (*(table->Type()->AsTableType()->IndexTypes()))[0]); + if ( ! zeek_key ) { - reporter->Error("ProcessStoreEvent Insert got %s when expecting table", data.get_type_name()); + reporter->Error("ProcessStoreEvent: failed to convert key"); return; } - // We sent this message. Ignore it. - //if ( insert.publisher() == storehandle->store_pid ) - // continue; - - // We currently use data_to_val here. It creates a bit of overhead, but makes the code easier. We could change - // this to manual unrolling in the future. - auto val = data_to_val(std::move(data), table->Type()); - // So we are just doing it manually - at least for now. - auto temptable = val->AsTable(); - auto temptableval = val->AsTableVal(); - HashKey* k; - TableEntryVal* entry; - auto c = temptable->InitForIteration(); - while ( (entry = temptable->NextEntry(k, c))) + if ( table->Type()->IsSet() ) { - auto lv = temptableval->RecoverIndex(k); - delete k; - Val* entry_key = lv->Length() == 1 ? lv->Index(0) : lv.get(); - - // note - at the moment this code creates a loop - // Fixme: expiry! - if ( temptableval->Type()->IsSet() ) - table->Assign(entry_key, nullptr); - else - table->Assign(entry_key, entry->Value()); + table->Assign(zeek_key.get(), nullptr); + return; } + + // it is a table + auto zeek_value = data_to_val(std::move(data), table->Type()->YieldType()); + if ( ! zeek_value ) + { + reporter->Error("ProcessStoreEvent: failed to convert value"); + return; + } + table->Assign(zeek_key.get(), zeek_value); } else if ( auto update = broker::store_event::update::make(msg) ) { - auto remstore = to_string(update.key()); - auto storehandle = broker_mgr->LookupStore(remstore); - if ( ! storehandle ) - return; - - auto table = storehandle->forward_to; - if ( ! table ) - return; - + auto key = update.key(); auto data = update.new_value(); - reporter->Warning("Update Data: %s->%s (%s)", to_string(update.old_value()).c_str(), to_string(update.new_value()).c_str(), update.new_value().get_type_name()); - if ( table->Type()->IsSet() && data.get_type() != broker::data::type::set ) + DBG_LOG(DBG_BROKER, "Update Data: %s->%s (%s)", to_string(update.old_value()).c_str(), to_string(update.new_value()).c_str(), update.new_value().get_type_name()); + if ( table->Type()->IsSet() && data.get_type() != broker::data::type::none ) { reporter->Error("ProcessStoreEvent Update got %s when expecting set", data.get_type_name()); return; } - else if ( ! table->Type()->IsSet() && data.get_type() != broker::data::type::table ) - { - reporter->Error("ProcessStoreEvent Update got %s when expecting table", data.get_type_name()); - return; - } // We sent this message. Ignore it. if ( update.publisher() == storehandle->store_pid ) return; - // We currently use data_to_val here. It creates a bit of overhead, but makes the code easier. We could change - // this to manual unrolling in the future. - auto val = data_to_val(std::move(data), table->Type()); - // So we are just doing it manually - at least for now. - auto temptable = val->AsTable(); - auto temptableval = val->AsTableVal(); - HashKey* k; - TableEntryVal* entry; - auto c = temptable->InitForIteration(); - while ( (entry = temptable->NextEntry(k, c))) + assert( table->Type()->AsTableType()->IndexTypes()->length() == 1 ); + auto zeek_key = data_to_val(std::move(key), (*(table->Type()->AsTableType()->IndexTypes()))[0]); + if ( ! zeek_key ) { - auto lv = temptableval->RecoverIndex(k); - delete k; - Val* entry_key = lv->Length() == 1 ? lv->Index(0) : lv.get(); - - // note - at the moment this code creates a loop - if ( temptableval->Type()->IsSet() ) - table->Assign(entry_key, nullptr); - else - table->Assign(entry_key, entry->Value()); + reporter->Error("ProcessStoreEvent: failed to convert key"); + return; } + + if ( table->Type()->IsSet() ) + { + table->Assign(zeek_key.get(), nullptr); + return; + } + + // it is a table + auto zeek_value = data_to_val(std::move(data), table->Type()->YieldType()); + if ( ! zeek_value ) + { + reporter->Error("ProcessStoreEvent: failed to convert value"); + return; + } + table->Assign(zeek_key.get(), zeek_value); } else if ( auto erase = broker::store_event::erase::make(msg) ) { - auto remstore = to_string(erase.key()); - auto storehandle = broker_mgr->LookupStore(remstore); - if ( ! storehandle ) + DBG_LOG(DBG_BROKER, "Erase for key %s", store_name.c_str()); + + // We sent this message. Ignore it. + if ( erase.publisher() == storehandle->store_pid ) return; - auto table = storehandle->forward_to; - if ( ! table ) + auto key = erase.key(); + assert( table->Type()->AsTableType()->IndexTypes()->length() == 1 ); + auto zeek_key = data_to_val(std::move(key), (*(table->Type()->AsTableType()->IndexTypes()))[0]); + if ( ! zeek_key ) + { + reporter->Error("ProcessStoreEvent: failed to convert key"); return; - - // I'm actually not sure we can get an erase - since we will never delete keys... - reporter->Warning("Erase for key %s", remstore.c_str()); + } + table->Delete(zeek_key.get()); } else { - reporter->Warning("Unhandled event type"); + reporter->Error("ProcessStoreEvent: Unhandled event type"); } } diff --git a/testing/btest/language/brokerstore.test b/testing/btest/language/brokerstore.test index b983e83851..c9fda7aa6f 100644 --- a/testing/btest/language/brokerstore.test +++ b/testing/btest/language/brokerstore.test @@ -24,6 +24,7 @@ event zeek_init() #add s["hi"]; #print "changing"; t["a"] = 2; + t["a"] = 3; t["b"] = 3; t["c"] = 4; t["whatever"] = 5; From f080c8c29495b8ebf29a7712ec9474cad8d9788e Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Wed, 3 Jun 2020 16:48:16 -0700 Subject: [PATCH 05/26] Broker changes: adopt to recent IntrusivePtr API changes --- src/Attr.cc | 4 ++-- src/Val.cc | 22 +++++++--------------- src/broker/Manager.cc | 37 ++++++++++++++++++++----------------- src/broker/Manager.h | 1 + 4 files changed, 30 insertions(+), 34 deletions(-) diff --git a/src/Attr.cc b/src/Attr.cc index 8126101afd..45a7296116 100644 --- a/src/Attr.cc +++ b/src/Attr.cc @@ -586,14 +586,14 @@ void Attributes::CheckAttr(Attr* a) break; } - if ( a->AttrExpr()->Type()->Tag() != TYPE_STRING ) + if ( a->GetExpr()->GetType()->Tag() != TYPE_STRING ) { Error("&broker_store must take a string argument"); break; } // Temporary since Broker does not support ListVals - and we cannot easily convert to set/vector - if ( type->AsTableType()->IndexTypes()->length() != 1 ) + if ( type->AsTableType()->IndexTypes().size() != 1 ) { Error("&broker_store only supports one-element set/table indexes"); } diff --git a/src/Val.cc b/src/Val.cc index 75070959a6..fc20cfc2d8 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -1488,12 +1488,12 @@ void TableVal::SetAttrs(IntrusivePtr a) if ( cf ) change_func = cf->GetExpr(); - auto bs = attrs->FindAttr(ATTR_BROKER_STORE); + auto bs = attrs->Find(ATTR_BROKER_STORE); if ( bs && broker_store.empty() ) // this does not mesh well with being updated several times { - IntrusivePtr c = bs->AttrExpr()->Eval(nullptr); + IntrusivePtr c = bs->GetExpr()->Eval(nullptr); assert(c); - assert(c->Type()->Tag() == TYPE_STRING); + assert(c->GetType()->Tag() == TYPE_STRING); broker_store = c->AsStringVal()->AsString()->CheckString(); broker_mgr->AddForwardedStore(broker_store, {NewRef{}, this}); } @@ -2113,7 +2113,6 @@ void TableVal::CallChangeFunc(const Val* index, in_change_func = false; } -<<<<<<< HEAD void TableVal::SendToStore(const Val* index, const Val* new_value, OnChangeType tpe) { if ( broker_store.empty() || ! index ) @@ -2129,7 +2128,7 @@ void TableVal::SendToStore(const Val* index, const Val* new_value, OnChangeType // we wither get passed the raw index_val - or a ListVal with exactly one element. // Since broker does not support ListVals, we have to unoll this in the second case. const Val* index_val; - if ( index->Type()->Tag() == TYPE_LIST ) + if ( index->GetType()->Tag() == TYPE_LIST ) { if ( index->AsListVal()->Length() != 1 ) { @@ -2137,7 +2136,7 @@ void TableVal::SendToStore(const Val* index, const Val* new_value, OnChangeType return; } - index_val = index->AsListVal()->Index(0); + index_val = index->AsListVal()->Idx(0).get(); } else { @@ -2187,10 +2186,7 @@ void TableVal::SendToStore(const Val* index, const Val* new_value, OnChangeType } } -IntrusivePtr TableVal::Delete(const Val* index) -======= IntrusivePtr TableVal::Remove(const Val& index) ->>>>>>> origin/master { auto k = MakeHashKey(index); TableEntryVal* v = k ? AsNonConstTable()->RemoveEntry(k.get()) : nullptr; @@ -2207,7 +2203,7 @@ IntrusivePtr TableVal::Remove(const Val& index) Modified(); if ( ! broker_store.empty() ) - SendToStore(index, nullptr, ELEMENT_REMOVED); + SendToStore(&index, nullptr, ELEMENT_REMOVED); if ( change_func ) CallChangeFunc(&index, va, ELEMENT_REMOVED); @@ -2237,14 +2233,10 @@ IntrusivePtr TableVal::Remove(const HashKey& k) if ( ( change_func && va ) || ( ! broker_store.empty() ) ) { auto index = table_hash->RecoverVals(k); -<<<<<<< HEAD if ( ! broker_store.empty() ) SendToStore(index.get(), nullptr, ELEMENT_REMOVED); if ( change_func && va ) - CallChangeFunc(index.get(), va.get(), ELEMENT_REMOVED); -======= - CallChangeFunc(index.get(), va, ELEMENT_REMOVED); ->>>>>>> origin/master + CallChangeFunc(index.get(), va, ELEMENT_REMOVED); } return va; diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index cfaa727791..d248fde171 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -967,7 +967,7 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) auto data = insert.value(); DBG_LOG(DBG_BROKER, "Insert Data: %s:%s (%s:%s)", to_string(insert.key()).c_str(), to_string(insert.value()).c_str(), insert.key().get_type_name(), insert.value().get_type_name()); - if ( table->Type()->IsSet() && data.get_type() != broker::data::type::none ) + if ( table->GetType()->IsSet() && data.get_type() != broker::data::type::none ) { reporter->Error("ProcessStoreEvent Insert got %s when expecting set", data.get_type_name()); return; @@ -978,28 +978,29 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) return; // FIXME: expiry! - assert( table->Type()->AsTableType()->IndexTypes()->length() == 1 ); - auto zeek_key = data_to_val(std::move(key), (*(table->Type()->AsTableType()->IndexTypes()))[0]); + const auto& its = table->GetType()->AsTableType()->IndexTypes(); + assert( its.size() == 1 ); + auto zeek_key = data_to_val(std::move(key), its[0].get()); if ( ! zeek_key ) { reporter->Error("ProcessStoreEvent: failed to convert key"); return; } - if ( table->Type()->IsSet() ) + if ( table->GetType()->IsSet() ) { - table->Assign(zeek_key.get(), nullptr); + table->Assign(zeek_key, nullptr); return; } // it is a table - auto zeek_value = data_to_val(std::move(data), table->Type()->YieldType()); + auto zeek_value = data_to_val(std::move(data), table->GetType()->Yield().get()); if ( ! zeek_value ) { reporter->Error("ProcessStoreEvent: failed to convert value"); return; } - table->Assign(zeek_key.get(), zeek_value); + table->Assign(zeek_key, zeek_value); } else if ( auto update = broker::store_event::update::make(msg) ) { @@ -1007,7 +1008,7 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) auto data = update.new_value(); DBG_LOG(DBG_BROKER, "Update Data: %s->%s (%s)", to_string(update.old_value()).c_str(), to_string(update.new_value()).c_str(), update.new_value().get_type_name()); - if ( table->Type()->IsSet() && data.get_type() != broker::data::type::none ) + if ( table->GetType()->IsSet() && data.get_type() != broker::data::type::none ) { reporter->Error("ProcessStoreEvent Update got %s when expecting set", data.get_type_name()); return; @@ -1017,28 +1018,29 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) if ( update.publisher() == storehandle->store_pid ) return; - assert( table->Type()->AsTableType()->IndexTypes()->length() == 1 ); - auto zeek_key = data_to_val(std::move(key), (*(table->Type()->AsTableType()->IndexTypes()))[0]); + const auto& its = table->GetType()->AsTableType()->IndexTypes(); + assert( its.size() == 1 ); + auto zeek_key = data_to_val(std::move(key), its[0].get()); if ( ! zeek_key ) { reporter->Error("ProcessStoreEvent: failed to convert key"); return; } - if ( table->Type()->IsSet() ) + if ( table->GetType()->IsSet() ) { - table->Assign(zeek_key.get(), nullptr); + table->Assign(zeek_key, nullptr); return; } // it is a table - auto zeek_value = data_to_val(std::move(data), table->Type()->YieldType()); + auto zeek_value = data_to_val(std::move(data), table->GetType()->Yield().get()); if ( ! zeek_value ) { reporter->Error("ProcessStoreEvent: failed to convert value"); return; } - table->Assign(zeek_key.get(), zeek_value); + table->Assign(zeek_key, zeek_value); } else if ( auto erase = broker::store_event::erase::make(msg) ) { @@ -1049,14 +1051,15 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) return; auto key = erase.key(); - assert( table->Type()->AsTableType()->IndexTypes()->length() == 1 ); - auto zeek_key = data_to_val(std::move(key), (*(table->Type()->AsTableType()->IndexTypes()))[0]); + const auto& its = table->GetType()->AsTableType()->IndexTypes(); + assert( its.size() == 1 ); + auto zeek_key = data_to_val(std::move(key), its[0].get()); if ( ! zeek_key ) { reporter->Error("ProcessStoreEvent: failed to convert key"); return; } - table->Delete(zeek_key.get()); + table->Remove(*zeek_key); } else { diff --git a/src/broker/Manager.h b/src/broker/Manager.h index 52292046e4..ca127e9a7f 100644 --- a/src/broker/Manager.h +++ b/src/broker/Manager.h @@ -25,6 +25,7 @@ class Frame; class Func; class VectorType; +class TableVal; namespace bro_broker { From 62f208086c3c0e079fb7493cadbfc5926730835e Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Fri, 5 Jun 2020 14:30:44 -0700 Subject: [PATCH 06/26] Update to recent broker changes. Specifically the store name is now part of the messages. --- aux/broker | 2 +- src/Val.cc | 9 ++++----- src/broker/Manager.cc | 37 +++++++++++++++++++++++++------------ 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/aux/broker b/aux/broker index 5c676e0fd4..8bba3c45c8 160000 --- a/aux/broker +++ b/aux/broker @@ -1 +1 @@ -Subproject commit 5c676e0fd42ecd5e36cbff19cca5b79a8f3b5a3d +Subproject commit 8bba3c45c85d99752dc6f8da9b40b768e3c39804 diff --git a/src/Val.cc b/src/Val.cc index 8c1cbe34db..37c936b073 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -1576,13 +1576,12 @@ bool TableVal::Assign(IntrusivePtr index, std::unique_ptr k, { auto change_index = index ? std::move(index) : RecreateIndex(k_copy); + const auto& v = old_entry_val ? old_entry_val->GetVal() : new_entry_val->GetVal(); + if ( ! broker_store.empty() ) - SendToStore(change_index.get(), new_val.get(), old_entry_val ? ELEMENT_CHANGED : ELEMENT_NEW); + SendToStore(change_index.get(), v.get(), old_entry_val ? ELEMENT_CHANGED : ELEMENT_NEW); if ( change_func ) - { - const auto& v = old_entry_val ? old_entry_val->GetVal() : new_entry_val->GetVal(); CallChangeFunc(change_index.get(), v, old_entry_val ? ELEMENT_CHANGED : ELEMENT_NEW); - } } delete old_entry_val; @@ -2138,7 +2137,7 @@ void TableVal::SendToStore(const Val* index, const Val* new_value, OnChangeType index_val = index; } - // FIXME: switch back to just storing tables directly in the broker store? + // FIXME: switch back to just storing tables directly in the broker store? // me which store a change came from - and this still seems to be missing from the store_events. (Or I am blind). auto broker_index = bro_broker::val_to_data(index_val); diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index d248fde171..fa4c586707 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -950,19 +950,16 @@ void Manager::Process() void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) { - auto topic_parts = broker::topic::split(topic); - const auto& store_name = topic_parts.back(); - - auto storehandle = broker_mgr->LookupStore(store_name); - if ( ! storehandle ) - return; - - auto table = storehandle->forward_to; - if ( ! table ) - return; - if ( auto insert = broker::store_event::insert::make(msg) ) { + auto storehandle = broker_mgr->LookupStore(insert.store_id()); + if ( ! storehandle ) + return; + + auto table = storehandle->forward_to; + if ( ! table ) + return; + auto key = insert.key(); auto data = insert.value(); @@ -1004,6 +1001,14 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) } else if ( auto update = broker::store_event::update::make(msg) ) { + auto storehandle = broker_mgr->LookupStore(update.store_id()); + if ( ! storehandle ) + return; + + auto table = storehandle->forward_to; + if ( ! table ) + return; + auto key = update.key(); auto data = update.new_value(); @@ -1044,7 +1049,15 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) } else if ( auto erase = broker::store_event::erase::make(msg) ) { - DBG_LOG(DBG_BROKER, "Erase for key %s", store_name.c_str()); + auto storehandle = broker_mgr->LookupStore(erase.store_id()); + if ( ! storehandle ) + return; + + auto table = storehandle->forward_to; + if ( ! table ) + return; + + DBG_LOG(DBG_BROKER, "Erase for key %s", erase.store_id().c_str()); // We sent this message. Ignore it. if ( erase.publisher() == storehandle->store_pid ) From 65c12ba6e95b83de2bdada532b5fc66fe6515d88 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Fri, 5 Jun 2020 16:14:51 -0700 Subject: [PATCH 07/26] Zeek/Brokerstore updates: first working end-to-end test This commit fixes a few more loose ends to actually make the Zeek Table<->brokerstore syncing work. This mostly slightly changes the TableVal assign/remove operators to prevent loops when a remote change arrives. The tests inserts a value into a table on the manager, and it pops out in a table on a clone - which is the easiest case. Timeouts are still not handled at all; the behavior when inserting into a clone is untested. --- src/Val.cc | 18 +-- src/Val.h | 12 +- src/broker/Manager.cc | 36 +++--- .../clone.out | 19 +++ .../broker/store/brokerstore-attr-simple.zeek | 109 ++++++++++++++++++ testing/btest/language/brokerstore.test | 34 ------ 6 files changed, 166 insertions(+), 62 deletions(-) create mode 100644 testing/btest/Baseline/broker.store.brokerstore-attr-simple/clone.out create mode 100644 testing/btest/broker/store/brokerstore-attr-simple.zeek delete mode 100644 testing/btest/language/brokerstore.test diff --git a/src/Val.cc b/src/Val.cc index 37c936b073..1f81d15398 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -1520,7 +1520,7 @@ void TableVal::CheckExpireAttr(attr_tag at) } } -bool TableVal::Assign(IntrusivePtr index, IntrusivePtr new_val) +bool TableVal::Assign(IntrusivePtr index, IntrusivePtr new_val, bool broker_forward) { auto k = MakeHashKey(*index); @@ -1530,7 +1530,7 @@ bool TableVal::Assign(IntrusivePtr index, IntrusivePtr new_val) return false; } - return Assign(std::move(index), std::move(k), std::move(new_val)); + return Assign(std::move(index), std::move(k), std::move(new_val), broker_forward); } bool TableVal::Assign(Val* index, Val* new_val) @@ -1539,7 +1539,7 @@ bool TableVal::Assign(Val* index, Val* new_val) } bool TableVal::Assign(IntrusivePtr index, std::unique_ptr k, - IntrusivePtr new_val) + IntrusivePtr new_val, bool broker_forward) { bool is_set = table_type->IsSet(); @@ -1572,16 +1572,18 @@ bool TableVal::Assign(IntrusivePtr index, std::unique_ptr k, Modified(); - if ( change_func || ( ! broker_store.empty() ) ) + if ( change_func || ( broker_forward && ! broker_store.empty() ) ) { auto change_index = index ? std::move(index) : RecreateIndex(k_copy); - const auto& v = old_entry_val ? old_entry_val->GetVal() : new_entry_val->GetVal(); if ( ! broker_store.empty() ) - SendToStore(change_index.get(), v.get(), old_entry_val ? ELEMENT_CHANGED : ELEMENT_NEW); + SendToStore(change_index.get(), new_entry_val->GetVal().get(), old_entry_val ? ELEMENT_CHANGED : ELEMENT_NEW); if ( change_func ) + { + const auto& v = old_entry_val ? old_entry_val->GetVal() : new_entry_val->GetVal(); CallChangeFunc(change_index.get(), v, old_entry_val ? ELEMENT_CHANGED : ELEMENT_NEW); + } } delete old_entry_val; @@ -2180,7 +2182,7 @@ void TableVal::SendToStore(const Val* index, const Val* new_value, OnChangeType } } -IntrusivePtr TableVal::Remove(const Val& index) +IntrusivePtr TableVal::Remove(const Val& index, bool broker_forward) { auto k = MakeHashKey(index); TableEntryVal* v = k ? AsNonConstTable()->RemoveEntry(k.get()) : nullptr; @@ -2196,7 +2198,7 @@ IntrusivePtr TableVal::Remove(const Val& index) Modified(); - if ( ! broker_store.empty() ) + if ( broker_forward && ! broker_store.empty() ) SendToStore(&index, nullptr, ELEMENT_REMOVED); if ( change_func ) CallChangeFunc(&index, va, ELEMENT_REMOVED); diff --git a/src/Val.h b/src/Val.h index 564a464850..4870bcb74d 100644 --- a/src/Val.h +++ b/src/Val.h @@ -772,9 +772,11 @@ public: * @param index The key to assign. * @param new_val The value to assign at the index. For a set, this * must be nullptr. + * @param broker_forward Controls if the value will be forwarded to attached + * broker stores. * @return True if the assignment type-checked. */ - bool Assign(IntrusivePtr index, IntrusivePtr new_val); + bool Assign(IntrusivePtr index, IntrusivePtr new_val, bool broker_forward = true); /** * Assigns a value at an associated index in the table (or in the @@ -784,10 +786,12 @@ public: * @param k A precomputed hash key to use. * @param new_val The value to assign at the index. For a set, this * must be nullptr. + * @param broker_forward Controls if the value will be forwarded to attached + * broker stores. * @return True if the assignment type-checked. */ bool Assign(IntrusivePtr index, std::unique_ptr k, - IntrusivePtr new_val); + IntrusivePtr new_val, bool broker_forward = true); // Returns true if the assignment typechecked, false if not. The // methods take ownership of new_val, but not of the index. If we're @@ -909,12 +913,14 @@ public: /** * Remove an element from the table and return it. * @param index The index to remove. + * @param broker_forward Controls if the remove operation will be forwarded to attached + * broker stores. * @return The value associated with the index if it exists, else nullptr. * For a sets that don't really contain associated values, a placeholder * value is returned to differentiate it from non-existent index (nullptr), * but otherwise has no meaning in relation to the set's contents. */ - IntrusivePtr Remove(const Val& index); + IntrusivePtr Remove(const Val& index, bool broker_forward = true); /** * Same as Remove(const Val&), but uses a precomputed hash key. diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index fa4c586707..ef672ceb41 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -963,17 +963,19 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) auto key = insert.key(); auto data = insert.value(); - DBG_LOG(DBG_BROKER, "Insert Data: %s:%s (%s:%s)", to_string(insert.key()).c_str(), to_string(insert.value()).c_str(), insert.key().get_type_name(), insert.value().get_type_name()); + // We sent this message. Ignore it. + if ( insert.publisher() == storehandle->store_pid ) + return; + + DBG_LOG(DBG_BROKER, "Store %s: Insert: %s:%s (%s:%s)", insert.store_id().c_str(), to_string(insert.key()).c_str(), to_string(insert.value()).c_str(), insert.key().get_type_name(), insert.value().get_type_name()); + + if ( table->GetType()->IsSet() && data.get_type() != broker::data::type::none ) { reporter->Error("ProcessStoreEvent Insert got %s when expecting set", data.get_type_name()); return; } - // We sent this message. Ignore it. - if ( insert.publisher() == storehandle->store_pid ) - return; - // FIXME: expiry! const auto& its = table->GetType()->AsTableType()->IndexTypes(); assert( its.size() == 1 ); @@ -986,7 +988,7 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) if ( table->GetType()->IsSet() ) { - table->Assign(zeek_key, nullptr); + table->Assign(zeek_key, nullptr, false); return; } @@ -997,7 +999,7 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) reporter->Error("ProcessStoreEvent: failed to convert value"); return; } - table->Assign(zeek_key, zeek_value); + table->Assign(zeek_key, zeek_value, false); } else if ( auto update = broker::store_event::update::make(msg) ) { @@ -1012,17 +1014,18 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) auto key = update.key(); auto data = update.new_value(); - DBG_LOG(DBG_BROKER, "Update Data: %s->%s (%s)", to_string(update.old_value()).c_str(), to_string(update.new_value()).c_str(), update.new_value().get_type_name()); + // We sent this message. Ignore it. + if ( update.publisher() == storehandle->store_pid ) + return; + + DBG_LOG(DBG_BROKER, "Store %s: Update: %s->%s (%s)", update.store_id().c_str(), to_string(update.old_value()).c_str(), to_string(update.new_value()).c_str(), update.new_value().get_type_name()); + if ( table->GetType()->IsSet() && data.get_type() != broker::data::type::none ) { reporter->Error("ProcessStoreEvent Update got %s when expecting set", data.get_type_name()); return; } - // We sent this message. Ignore it. - if ( update.publisher() == storehandle->store_pid ) - return; - const auto& its = table->GetType()->AsTableType()->IndexTypes(); assert( its.size() == 1 ); auto zeek_key = data_to_val(std::move(key), its[0].get()); @@ -1034,7 +1037,7 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) if ( table->GetType()->IsSet() ) { - table->Assign(zeek_key, nullptr); + table->Assign(zeek_key, nullptr, false); return; } @@ -1045,7 +1048,7 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) reporter->Error("ProcessStoreEvent: failed to convert value"); return; } - table->Assign(zeek_key, zeek_value); + table->Assign(zeek_key, zeek_value, false); } else if ( auto erase = broker::store_event::erase::make(msg) ) { @@ -1057,13 +1060,12 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) if ( ! table ) return; - DBG_LOG(DBG_BROKER, "Erase for key %s", erase.store_id().c_str()); - // We sent this message. Ignore it. if ( erase.publisher() == storehandle->store_pid ) return; auto key = erase.key(); + DBG_LOG(DBG_BROKER, "Store %s: Erase key %s", erase.store_id().c_str(), to_string(key).c_str()); const auto& its = table->GetType()->AsTableType()->IndexTypes(); assert( its.size() == 1 ); auto zeek_key = data_to_val(std::move(key), its[0].get()); @@ -1072,7 +1074,7 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) reporter->Error("ProcessStoreEvent: failed to convert key"); return; } - table->Remove(*zeek_key); + table->Remove(*zeek_key, false); } else { diff --git a/testing/btest/Baseline/broker.store.brokerstore-attr-simple/clone.out b/testing/btest/Baseline/broker.store.brokerstore-attr-simple/clone.out new file mode 100644 index 0000000000..b2bbfd2600 --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-attr-simple/clone.out @@ -0,0 +1,19 @@ +Peer added +{ +[b] = 3, +[whatever] = 5, +[a] = 3 +} +{ +hi +} +{ +[b] = [a=2, b=d, c={ +elem1, +elem2 +}], +[a] = [a=1, b=c, c={ +elem1, +elem2 +}] +} diff --git a/testing/btest/broker/store/brokerstore-attr-simple.zeek b/testing/btest/broker/store/brokerstore-attr-simple.zeek new file mode 100644 index 0000000000..ac2bfd2233 --- /dev/null +++ b/testing/btest/broker/store/brokerstore-attr-simple.zeek @@ -0,0 +1,109 @@ +# @TEST-PORT: BROKER_PORT + +# @TEST-EXEC: btest-bg-run master "zeek -B broker -b ../master.zeek >../master.out" +# @TEST-EXEC: btest-bg-run clone "zeek -B broker -b ../clone.zeek >../clone.out" +# @TEST-EXEC: btest-bg-wait 15 +# +# @TEST-EXEC: btest-diff clone.out + +@TEST-START-FILE master.zeek +redef exit_only_after_terminate = T; + +module TestModule; + +global tablestore: opaque of Broker::Store; +global setstore: opaque of Broker::Store; +global recordstore: opaque of Broker::Store; + +type testrec: record { + a: count; + b: string; + c: set[string]; +}; + +global t: table[string] of count &broker_store="table"; +global s: set[string] &broker_store="set"; +global r: table[string] of testrec &broker_store="rec"; + +event zeek_init() + { + Broker::listen("127.0.0.1", to_port(getenv("BROKER_PORT"))); + tablestore = Broker::create_master("table"); + setstore = Broker::create_master("set"); + recordstore = Broker::create_master("rec"); + } + +event insert_stuff() + { + print "Inserting stuff"; + t["a"] = 5; + delete t["a"]; + add s["hi"]; + t["a"] = 2; + t["a"] = 3; + t["b"] = 3; + t["c"] = 4; + t["whatever"] = 5; + delete t["c"]; + r["a"] = testrec($a=1, $b="b", $c=set("elem1", "elem2")); + r["a"] = testrec($a=1, $b="c", $c=set("elem1", "elem2")); + r["b"] = testrec($a=2, $b="d", $c=set("elem1", "elem2")); + print t; + print s; + print r; + } + +event Broker::peer_added(endpoint: Broker::EndpointInfo, msg: string) + { + print "Peer added ", endpoint; + schedule 3secs { insert_stuff() }; + } + +event Broker::peer_lost(endpoint: Broker::EndpointInfo, msg: string) + { + terminate(); + } + +@TEST-END-FILE + +@TEST-START-FILE clone.zeek +redef exit_only_after_terminate = T; + +module TestModule; + +global tablestore: opaque of Broker::Store; +global setstore: opaque of Broker::Store; +global recordstore: opaque of Broker::Store; + +type testrec: record { + a: count; + b: string; + c: set[string]; +}; + +global t: table[string] of count &broker_store="table"; +global s: set[string] &broker_store="set"; +global r: table[string] of testrec &broker_store="rec"; + +event zeek_init() + { + Broker::peer("127.0.0.1", to_port(getenv("BROKER_PORT"))); + } + +event dump_tables() + { + print t; + print s; + print r; + terminate(); + } + +event Broker::peer_added(endpoint: Broker::EndpointInfo, msg: string) + { + print "Peer added"; + tablestore = Broker::create_clone("table"); + setstore = Broker::create_clone("set"); + recordstore = Broker::create_clone("rec"); + schedule 5secs { dump_tables() }; + } +@TEST-END-FILE diff --git a/testing/btest/language/brokerstore.test b/testing/btest/language/brokerstore.test deleted file mode 100644 index c9fda7aa6f..0000000000 --- a/testing/btest/language/brokerstore.test +++ /dev/null @@ -1,34 +0,0 @@ -# @TEST-EXEC: zeek %INPUT >output -# @TEST-EXEC: btest-diff output - -redef exit_only_after_terminate = T; - -module TestModule; - -global tablestore: opaque of Broker::Store; -#global tabletwostore: opaque of Broker::Store; -global setstore: opaque of Broker::Store; - -global t: table[string] of count &broker_store="table"; -#global ct: table[string, string] of count &broker_store="table2"; -global s: set[string] &broker_store="set"; - -event zeek_init() - { - tablestore = Broker::create_master("table"); - #tabletwostore = Broker::create_master("table2"); - setstore = Broker::create_master("set"); - print "inserting"; - t["a"] = 5; - delete t["a"]; - #add s["hi"]; - #print "changing"; - t["a"] = 2; - t["a"] = 3; - t["b"] = 3; - t["c"] = 4; - t["whatever"] = 5; - #print "deleting"; - #delete t["a"]; - #delete s["hi"]; - } From 58468ab39fb6a4bdd193a814fb8757710c9ec58f Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Mon, 15 Jun 2020 14:04:53 -0700 Subject: [PATCH 08/26] Zeek/Brokerstore updates: add test that includes updates from clones --- .../clonetwo.out | 19 +++ .../master.out | 38 +++++ .../broker/store/brokerstore-attr-clone.zeek | 159 ++++++++++++++++++ 3 files changed, 216 insertions(+) create mode 100644 testing/btest/Baseline/broker.store.brokerstore-attr-clone/clonetwo.out create mode 100644 testing/btest/Baseline/broker.store.brokerstore-attr-clone/master.out create mode 100644 testing/btest/broker/store/brokerstore-attr-clone.zeek diff --git a/testing/btest/Baseline/broker.store.brokerstore-attr-clone/clonetwo.out b/testing/btest/Baseline/broker.store.brokerstore-attr-clone/clonetwo.out new file mode 100644 index 0000000000..b2bbfd2600 --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-attr-clone/clonetwo.out @@ -0,0 +1,19 @@ +Peer added +{ +[b] = 3, +[whatever] = 5, +[a] = 3 +} +{ +hi +} +{ +[b] = [a=2, b=d, c={ +elem1, +elem2 +}], +[a] = [a=1, b=c, c={ +elem1, +elem2 +}] +} diff --git a/testing/btest/Baseline/broker.store.brokerstore-attr-clone/master.out b/testing/btest/Baseline/broker.store.brokerstore-attr-clone/master.out new file mode 100644 index 0000000000..b18d9195da --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-attr-clone/master.out @@ -0,0 +1,38 @@ +Peer added +Peer added +{ +[b] = 3, +[whatever] = 5, +[a] = 3 +} +{ +hi +} +{ +[b] = [a=2, b=d, c={ +elem1, +elem2 +}], +[a] = [a=1, b=c, c={ +elem1, +elem2 +}] +} +{ +[b] = 3, +[whatever] = 5, +[a] = 3 +} +{ +hi +} +{ +[b] = [a=2, b=d, c={ +elem1, +elem2 +}], +[a] = [a=1, b=c, c={ +elem1, +elem2 +}] +} diff --git a/testing/btest/broker/store/brokerstore-attr-clone.zeek b/testing/btest/broker/store/brokerstore-attr-clone.zeek new file mode 100644 index 0000000000..e618196f0d --- /dev/null +++ b/testing/btest/broker/store/brokerstore-attr-clone.zeek @@ -0,0 +1,159 @@ +# Start master and two clones. One clone changes table and the change ends up in master + other clone. + +# @TEST-PORT: BROKER_PORT + +# @TEST-EXEC: btest-bg-run master "zeek -B broker -b ../master.zeek >../master.out" +# @TEST-EXEC: btest-bg-run cloneone "zeek -B broker -b ../cloneone.zeek >../cloneone.out" +# @TEST-EXEC: btest-bg-run clonetwo "zeek -B broker -b ../clonetwo.zeek >../clonetwo.out" +# @TEST-EXEC: btest-bg-wait 15 +# +# @TEST-EXEC: btest-diff master.out +# @TEST-EXEC: btest-diff clonetwo.out + +@TEST-START-FILE master.zeek +redef exit_only_after_terminate = T; + +module TestModule; + +global tablestore: opaque of Broker::Store; +global setstore: opaque of Broker::Store; +global recordstore: opaque of Broker::Store; + +type testrec: record { + a: count; + b: string; + c: set[string]; +}; + +global t: table[string] of count &broker_store="table"; +global s: set[string] &broker_store="set"; +global r: table[string] of testrec &broker_store="rec"; + +event zeek_init() + { + Broker::listen("127.0.0.1", to_port(getenv("BROKER_PORT"))); + tablestore = Broker::create_master("table"); + setstore = Broker::create_master("set"); + recordstore = Broker::create_master("rec"); + } + +event dump_tables() + { + print t; + print s; + print r; + } + +event Broker::peer_added(endpoint: Broker::EndpointInfo, msg: string) + { + print "Peer added "; + schedule 5secs { dump_tables() }; + } + +event Broker::peer_lost(endpoint: Broker::EndpointInfo, msg: string) + { + terminate(); + } +@TEST-END-FILE + +@TEST-START-FILE cloneone.zeek +redef exit_only_after_terminate = T; + +module TestModule; + +global tablestore: opaque of Broker::Store; +global setstore: opaque of Broker::Store; +global recordstore: opaque of Broker::Store; + +type testrec: record { + a: count; + b: string; + c: set[string]; +}; + +global t: table[string] of count &broker_store="table"; +global s: set[string] &broker_store="set"; +global r: table[string] of testrec &broker_store="rec"; + +event zeek_init() + { + Broker::peer("127.0.0.1", to_port(getenv("BROKER_PORT"))); + } + +event send_stuff_over() + { + print "Inserting stuff"; + t["a"] = 5; + delete t["a"]; + add s["hi"]; + t["a"] = 2; + t["a"] = 3; + t["b"] = 3; + t["c"] = 4; + t["whatever"] = 5; + delete t["c"]; + r["a"] = testrec($a=1, $b="b", $c=set("elem1", "elem2")); + r["a"] = testrec($a=1, $b="c", $c=set("elem1", "elem2")); + r["b"] = testrec($a=2, $b="d", $c=set("elem1", "elem2")); + print t; + print s; + print r; + } + +event killmeplease() + { + terminate(); + } + +event Broker::peer_added(endpoint: Broker::EndpointInfo, msg: string) + { + print "Peer added", endpoint; + tablestore = Broker::create_clone("table"); + setstore = Broker::create_clone("set"); + recordstore = Broker::create_clone("rec"); + schedule 2secs { send_stuff_over() }; + schedule 5secs { killmeplease() }; + } +@TEST-END-FILE + +@TEST-START-FILE clonetwo.zeek +redef exit_only_after_terminate = T; + +module TestModule; + +global tablestore: opaque of Broker::Store; +global setstore: opaque of Broker::Store; +global recordstore: opaque of Broker::Store; + +type testrec: record { + a: count; + b: string; + c: set[string]; +}; + +global t: table[string] of count &broker_store="table"; +global s: set[string] &broker_store="set"; +global r: table[string] of testrec &broker_store="rec"; + +event zeek_init() + { + Broker::peer("127.0.0.1", to_port(getenv("BROKER_PORT"))); + } + +event dump_tables() + { + print t; + print s; + print r; + terminate(); + } + +event Broker::peer_added(endpoint: Broker::EndpointInfo, msg: string) + { + print "Peer added"; + tablestore = Broker::create_clone("table"); + setstore = Broker::create_clone("set"); + recordstore = Broker::create_clone("rec"); + schedule 5secs { dump_tables() }; + } +@TEST-END-FILE From 09119ae69ddfd0fe5144bb88a834ff1da7ad2286 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Wed, 24 Jun 2020 16:49:18 -0700 Subject: [PATCH 09/26] Zeek/Brokerstore updates: expiration Expiration is done separately by the manager, the clones, and the brokerstore. But - it should happen roughly at the same time. --- src/Val.cc | 42 +++- src/Val.h | 2 +- src/broker/Manager.cc | 16 ++ src/broker/Store.h | 19 ++ src/broker/store.bif | 33 +--- .../clone.out | 19 ++ .../broker/store/brokerstore-attr-expire.zeek | 181 ++++++++++++++++++ 7 files changed, 281 insertions(+), 31 deletions(-) create mode 100644 testing/btest/Baseline/broker.store.brokerstore-attr-expire/clone.out create mode 100644 testing/btest/broker/store/brokerstore-attr-expire.zeek diff --git a/src/Val.cc b/src/Val.cc index b4c3b9c1ad..5d19a9dc77 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -1488,7 +1488,7 @@ void TableVal::SetAttrs(IntrusivePtr a) { IntrusivePtr c = bs->GetExpr()->Eval(nullptr); assert(c); - assert(c->GetType()->Tag() == TYPE_STRING); + assert(c->GetType()->Tag() == zeek::TYPE_STRING); broker_store = c->AsStringVal()->AsString()->CheckString(); broker_mgr->AddForwardedStore(broker_store, {NewRef{}, this}); } @@ -1578,7 +1578,7 @@ bool TableVal::Assign(IntrusivePtr index, std::unique_ptr k, : RecreateIndex(k_copy); if ( ! broker_store.empty() ) - SendToStore(change_index.get(), new_entry_val->GetVal().get(), old_entry_val ? ELEMENT_CHANGED : ELEMENT_NEW); + SendToStore(change_index.get(), new_entry_val, old_entry_val ? ELEMENT_CHANGED : ELEMENT_NEW); if ( change_func ) { const auto& v = old_entry_val ? old_entry_val->GetVal() : new_entry_val->GetVal(); @@ -2110,7 +2110,7 @@ void TableVal::CallChangeFunc(const Val* index, in_change_func = false; } -void TableVal::SendToStore(const Val* index, const Val* new_value, OnChangeType tpe) +void TableVal::SendToStore(const Val* index, const TableEntryVal* new_entry_val, OnChangeType tpe) { if ( broker_store.empty() || ! index ) return; @@ -2125,7 +2125,7 @@ void TableVal::SendToStore(const Val* index, const Val* new_value, OnChangeType // we wither get passed the raw index_val - or a ListVal with exactly one element. // Since broker does not support ListVals, we have to unoll this in the second case. const Val* index_val; - if ( index->GetType()->Tag() == TYPE_LIST ) + if ( index->GetType()->Tag() == zeek::TYPE_LIST ) { if ( index->AsListVal()->Length() != 1 ) { @@ -2154,27 +2154,55 @@ void TableVal::SendToStore(const Val* index, const Val* new_value, OnChangeType { case ELEMENT_NEW: case ELEMENT_CHANGED: + { + broker::optional expiry; + + auto expire_time = GetExpireTime(); + if ( expire_time == 0 ) + // Entry is set to immediately expire. Let's not forward it. + break; + + if ( expire_time > 0 ) + { + if ( attrs->Find(zeek::detail::ATTR_EXPIRE_CREATE) ) + { + // for create expiry, we have to substract the already elapsed time from the expiry. + auto e = expire_time - (network_time - new_entry_val->ExpireAccessTime()); + if ( e <= 0 ) + // element already expired? Let's not insert it. + break; + expiry = bro_broker::convert_expiry(e); + } + else + expiry = bro_broker::convert_expiry(expire_time); + } if ( table_type->IsSet() ) - handle->store.put(std::move(*broker_index), broker::data()); + handle->store.put(std::move(*broker_index), broker::data(), expiry); else { - if ( ! new_value ) + if ( ! new_entry_val ) { builtin_error("did not receive new value for broker-store send operation"); return; } + auto new_value = new_entry_val->GetVal().get(); auto broker_val = bro_broker::val_to_data(new_value); if ( ! broker_val ) { builtin_error("invalid Broker data conversation for table value"); return; } - handle->store.put(std::move(*broker_index), std::move(*broker_val)); + handle->store.put(std::move(*broker_index), std::move(*broker_val), expiry); } break; + } case ELEMENT_REMOVED: handle->store.erase(std::move(*broker_index)); break; + case ELEMENT_EXPIRED: + // we do nothing here. The broker store does its own expiration - so the element + // should expire at about the same time. + break; } } catch ( InterpreterException& e ) diff --git a/src/Val.h b/src/Val.h index ed5bbf5692..bbd1fb7e2d 100644 --- a/src/Val.h +++ b/src/Val.h @@ -1075,7 +1075,7 @@ protected: OnChangeType tpe); // Sends data on to backing Broker Store - void SendToStore(const Val* index, const Val* new_value, OnChangeType tpe); + void SendToStore(const Val* index, const TableEntryVal* new_entry_val, OnChangeType tpe); IntrusivePtr DoClone(CloneState* state) override; diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index 9659a146d5..18403b9f9c 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -1076,6 +1076,22 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) } table->Remove(*zeek_key, false); } + else if ( auto expire = broker::store_event::expire::make(msg) ) + { + // We just ignore expirys - expiring information on the Zeek side is handled by Zeek itself. +#ifdef DEBUG + // let's only debug log for stores that we know. + auto storehandle = broker_mgr->LookupStore(expire.store_id()); + if ( ! storehandle ) + return; + + auto table = storehandle->forward_to; + if ( ! table ) + return; + + DBG_LOG(DBG_BROKER, "Store %s: Store expired key %s", expire.store_id().c_str(), to_string(expire.key()).c_str()); +#endif /* DEBUG */ + } else { reporter->Error("ProcessStoreEvent: Unhandled event type"); diff --git a/src/broker/Store.h b/src/broker/Store.h index 48bb4c36a6..46787c0525 100644 --- a/src/broker/Store.h +++ b/src/broker/Store.h @@ -46,6 +46,25 @@ inline IntrusivePtr query_result(IntrusivePtr data) return rval; } +/** + * Convert an expiry from a double (used to Zeek) to the format required by Broker + * @param e: expire interval as double; 0 if no expiry + * @return expire interval in broker format + */ +static broker::optional convert_expiry(double e) + { + broker::optional ts; + + if ( e ) + { + broker::timespan x; + broker::convert(e, x); + ts = x; + } + + return ts; + } + /** * Used for asynchronous data store queries which use "when" statements. */ diff --git a/src/broker/store.bif b/src/broker/store.bif index 68262f667b..6f1fdca8fd 100644 --- a/src/broker/store.bif +++ b/src/broker/store.bif @@ -7,19 +7,6 @@ #include "broker/Data.h" #include "Trigger.h" -static broker::optional prepare_expiry(double e) - { - broker::optional ts; - - if ( e ) - { - broker::timespan x; - broker::convert(e, x); - ts = x; - } - - return ts; - } %%} module Broker; @@ -262,7 +249,7 @@ function Broker::__put_unique%(h: opaque of Broker::Store, handle->store); auto req_id = handle->proxy.put_unique(std::move(*key), std::move(*val), - prepare_expiry(e)); + bro_broker::convert_expiry(e)); broker_mgr->TrackStoreQuery(handle, req_id, cb); return nullptr; @@ -384,7 +371,7 @@ function Broker::__put%(h: opaque of Broker::Store, return val_mgr->False(); } - handle->store.put(std::move(*key), std::move(*val), prepare_expiry(e)); + handle->store.put(std::move(*key), std::move(*val), bro_broker::convert_expiry(e)); return val_mgr->True(); %} @@ -435,7 +422,7 @@ function Broker::__increment%(h: opaque of Broker::Store, k: any, a: any, } handle->store.increment(std::move(*key), std::move(*amount), - prepare_expiry(e)); + bro_broker::convert_expiry(e)); return val_mgr->True(); %} @@ -464,7 +451,7 @@ function Broker::__decrement%(h: opaque of Broker::Store, k: any, a: any, return val_mgr->False(); } - handle->store.decrement(std::move(*key), std::move(*amount), prepare_expiry(e)); + handle->store.decrement(std::move(*key), std::move(*amount), bro_broker::convert_expiry(e)); return val_mgr->True(); %} @@ -493,7 +480,7 @@ function Broker::__append%(h: opaque of Broker::Store, k: any, s: any, return val_mgr->False(); } - handle->store.append(std::move(*key), std::move(*str), prepare_expiry(e)); + handle->store.append(std::move(*key), std::move(*str), bro_broker::convert_expiry(e)); return val_mgr->True(); %} @@ -523,7 +510,7 @@ function Broker::__insert_into_set%(h: opaque of Broker::Store, k: any, i: any, } handle->store.insert_into(std::move(*key), std::move(*idx), - prepare_expiry(e)); + bro_broker::convert_expiry(e)); return val_mgr->True(); %} @@ -560,7 +547,7 @@ function Broker::__insert_into_table%(h: opaque of Broker::Store, k: any, } handle->store.insert_into(std::move(*key), std::move(*idx), - std::move(*val), prepare_expiry(e)); + std::move(*val), bro_broker::convert_expiry(e)); return val_mgr->True(); %} @@ -590,7 +577,7 @@ function Broker::__remove_from%(h: opaque of Broker::Store, k: any, i: any, } handle->store.remove_from(std::move(*key), std::move(*idx), - prepare_expiry(e)); + bro_broker::convert_expiry(e)); return val_mgr->True(); %} @@ -619,7 +606,7 @@ function Broker::__push%(h: opaque of Broker::Store, k: any, v: any, return val_mgr->False(); } - handle->store.push(std::move(*key), std::move(*val), prepare_expiry(e)); + handle->store.push(std::move(*key), std::move(*val), bro_broker::convert_expiry(e)); return val_mgr->True(); %} @@ -640,7 +627,7 @@ function Broker::__pop%(h: opaque of Broker::Store, k: any, e: interval%): bool return val_mgr->False(); } - handle->store.pop(std::move(*key), prepare_expiry(e)); + handle->store.pop(std::move(*key), bro_broker::convert_expiry(e)); return val_mgr->True(); %} diff --git a/testing/btest/Baseline/broker.store.brokerstore-attr-expire/clone.out b/testing/btest/Baseline/broker.store.brokerstore-attr-expire/clone.out new file mode 100644 index 0000000000..100aaba603 --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-attr-expire/clone.out @@ -0,0 +1,19 @@ +Peer added +Expiring s: expire_first +Expiring t: b +Expiring t: whatever +Expiring t: a +Expiring r: recb +Expiring s: expire_later +Expiring t: expire_later_in_t_not_with_a +Expiring r: reca +{ + +} +{ + +} +{ + +} +terminating diff --git a/testing/btest/broker/store/brokerstore-attr-expire.zeek b/testing/btest/broker/store/brokerstore-attr-expire.zeek new file mode 100644 index 0000000000..32ca097de7 --- /dev/null +++ b/testing/btest/broker/store/brokerstore-attr-expire.zeek @@ -0,0 +1,181 @@ +# @TEST-PORT: BROKER_PORT + +# @TEST-EXEC: btest-bg-run master "zeek -B broker -b ../master.zeek >../master.out" +# @TEST-EXEC: btest-bg-run clone "zeek -B broker -b ../clone.zeek >../clone.out" +# @TEST-EXEC: btest-bg-wait 20 +# +# @TEST-EXEC: btest-diff clone.out + +@TEST-START-FILE master.zeek +redef exit_only_after_terminate = T; +redef table_expire_interval = 0.5sec; + +module TestModule; + +global start_time: time; + +function time_past(): interval + { + return network_time() - start_time; + } + +global tablestore: opaque of Broker::Store; +global setstore: opaque of Broker::Store; +global recordstore: opaque of Broker::Store; + +type testrec: record { + a: count; + b: string; + c: set[string]; +}; + +function expire_t(tbl: any, idx: string): interval + { + print fmt("Expiring t: %s", idx); + return 0sec; + } + +function expire_s(tbl: any, idx: string): interval + { + print fmt("Expiring s: %s", idx); + return 0sec; + } + +function expire_r(tbl: any, idx: string): interval + { + print fmt("Expiring r: %s", idx); + return 0sec; + } + +function print_keys() + { + print "Printing keys"; + when ( local s = Broker::keys(tablestore) ) + { + print "keys", s; + } + timeout 2sec + { + print fmt(""); + } + } + +global t: table[string] of count &broker_store="table" &create_expire=4sec &expire_func=expire_t; +global s: table[string] of count &broker_store="set" &write_expire=3sec &expire_func=expire_s; +global r: table[string] of testrec &broker_store="rec" &read_expire=5sec &expire_func=expire_r; + +event zeek_init() + { + Broker::listen("127.0.0.1", to_port(getenv("BROKER_PORT"))); + tablestore = Broker::create_master("table"); + setstore = Broker::create_master("set"); + recordstore = Broker::create_master("rec"); + } + +event update_stuff() + { + t["a"] = 3; + t["expire_later_in_t_not_with_a"] = 4; + s["expire_later"] = 2; + r["reca"] = testrec($a=1, $b="c", $c=set("elem1", "elem2")); + } + +event insert_stuff() + { + print "Inserting stuff"; + start_time = network_time(); + t["a"] = 5; + delete t["a"]; + s["expire_first"] = 0; + s["expire_later"] = 1; + t["a"] = 2; + t["b"] = 3; + t["whatever"] = 5; + r["reca"] = testrec($a=1, $b="b", $c=set("elem1", "elem2")); + r["recb"] = testrec($a=2, $b="d", $c=set("elem1", "elem2")); + print t; + print s; + print r; + schedule 1.5sec { update_stuff() }; + } + +event Broker::peer_added(endpoint: Broker::EndpointInfo, msg: string) + { + print "Peer added ", endpoint; + schedule 3secs { insert_stuff() }; + } + +event terminate_me() + { + print "Terminating"; + terminate(); + } + +event Broker::peer_lost(endpoint: Broker::EndpointInfo, msg: string) + { + print_keys(); + schedule 3secs { terminate_me() }; + } +@TEST-END-FILE + +@TEST-START-FILE clone.zeek +redef exit_only_after_terminate = T; +redef table_expire_interval = 0.5sec; + +module TestModule; + +global tablestore: opaque of Broker::Store; +global setstore: opaque of Broker::Store; +global recordstore: opaque of Broker::Store; + +type testrec: record { + a: count; + b: string; + c: set[string]; +}; + +function expire_t(tbl: any, idx: string): interval + { + print fmt("Expiring t: %s", idx); + return 0sec; + } + +function expire_s(tbl: any, idx: string): interval + { + print fmt("Expiring s: %s", idx); + return 0sec; + } + +function expire_r(tbl: any, idx: string): interval + { + print fmt("Expiring r: %s", idx); + return 0sec; + } + +global t: table[string] of count &broker_store="table" &create_expire=4sec &expire_func=expire_t; +global s: table[string] of count &broker_store="set" &write_expire=3sec &expire_func=expire_s; +global r: table[string] of testrec &broker_store="rec" &read_expire=5sec &expire_func=expire_r; + +event zeek_init() + { + Broker::peer("127.0.0.1", to_port(getenv("BROKER_PORT"))); + } + +event dump_tables() + { + print t; + print s; + print r; + print "terminating"; + terminate(); + } + +event Broker::peer_added(endpoint: Broker::EndpointInfo, msg: string) + { + print "Peer added"; + tablestore = Broker::create_clone("table"); + setstore = Broker::create_clone("set"); + recordstore = Broker::create_clone("rec"); + schedule 15secs { dump_tables() }; + } +@TEST-END-FILE From b027b69f5d2f4a02f858a634af6360e7183501b6 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Thu, 25 Jun 2020 19:28:35 -0700 Subject: [PATCH 10/26] Brokerstore<->Tables: attribute conflicts Makes some attributes conflict with each other. This also needed the test to change. The test is a bit flaky - but I can, for the heck of it, not figure out why. I am punting that for the future after spending a few hours on it. --- src/Attr.cc | 23 ++++++++- src/Val.cc | 40 +++++++++++----- src/Val.h | 4 +- .../broker/store/brokerstore-attr-expire.zeek | 48 +++++++++---------- 4 files changed, 74 insertions(+), 41 deletions(-) diff --git a/src/Attr.cc b/src/Attr.cc index bf8ba410a0..5b56fe1a07 100644 --- a/src/Attr.cc +++ b/src/Attr.cc @@ -458,6 +458,13 @@ void Attributes::CheckAttr(Attr* a) break; case ATTR_EXPIRE_READ: + { + if ( Find(ATTR_BROKER_STORE) ) + { + Error("&broker_store and &read_expire cannot be used simultaneously"); + } + } + // fallthrough case ATTR_EXPIRE_WRITE: case ATTR_EXPIRE_CREATE: { @@ -536,6 +543,11 @@ void Attributes::CheckAttr(Attr* a) if ( ! e_ft->CheckArgs(&expected_args) ) Error("&expire_func argument type clash"); + + if ( Find(ATTR_BROKER_STORE ) ) + { + Error("&broker_store and &expire_func cannot be used simultaneously"); + } } break; @@ -623,9 +635,16 @@ void Attributes::CheckAttr(Attr* a) { Error("&broker_store only supports one-element set/table indexes"); } - + if ( Find(ATTR_EXPIRE_FUNC ) ) + { + Error("&broker_store and &expire_func cannot be used simultaneously"); + } + if ( Find(ATTR_EXPIRE_READ) ) + { + Error("&broker_store and &read_expire cannot be used simultaneously"); + } + break; } - case ATTR_TRACKED: // FIXME: Check here for global ID? break; diff --git a/src/Val.cc b/src/Val.cc index 5d19a9dc77..79fcf3fc0d 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -1577,12 +1577,12 @@ bool TableVal::Assign(IntrusivePtr index, std::unique_ptr k, auto change_index = index ? std::move(index) : RecreateIndex(k_copy); - if ( ! broker_store.empty() ) + if ( broker_forward && ! broker_store.empty() ) SendToStore(change_index.get(), new_entry_val, old_entry_val ? ELEMENT_CHANGED : ELEMENT_NEW); if ( change_func ) { const auto& v = old_entry_val ? old_entry_val->GetVal() : new_entry_val->GetVal(); - CallChangeFunc(change_index.get(), v, old_entry_val ? ELEMENT_CHANGED : ELEMENT_NEW); + CallChangeFunc(change_index, v, old_entry_val ? ELEMENT_CHANGED : ELEMENT_NEW); } } @@ -2049,7 +2049,7 @@ IntrusivePtr TableVal::RecreateIndex(const HashKey& k) const return table_hash->RecoverVals(k); } -void TableVal::CallChangeFunc(const Val* index, +void TableVal::CallChangeFunc(const IntrusivePtr& index, const IntrusivePtr& old_value, OnChangeType tpe) { @@ -2073,10 +2073,14 @@ void TableVal::CallChangeFunc(const Val* index, } const Func* f = thefunc->AsFunc(); - auto lv = index->AsListVal(); - zeek::Args vl; - vl.reserve(2 + lv->Length() + table_type->IsTable()); + + // we either get passed the raw index_val - or a ListVal with exactly one element. + if ( index->GetType()->Tag() == zeek::TYPE_LIST ) + vl.reserve(2 + index->AsListVal()->Length() + table_type->IsTable()); + else + vl.reserve(3 + table_type->IsTable()); + vl.emplace_back(NewRef{}, this); switch ( tpe ) @@ -2094,8 +2098,14 @@ void TableVal::CallChangeFunc(const Val* index, vl.emplace_back(zeek::BifType::Enum::TableChange->GetVal(BifEnum::TableChange::TABLE_ELEMENT_EXPIRED)); } - for ( const auto& v : lv->Vals() ) - vl.emplace_back(v); + + if ( index->GetType()->Tag() == zeek::TYPE_LIST ) + { + for ( const auto& v : index->AsListVal()->Vals() ) + vl.emplace_back(v); + } + else + vl.emplace_back(index); if ( table_type->IsTable() ) vl.emplace_back(old_value); @@ -2122,7 +2132,7 @@ void TableVal::SendToStore(const Val* index, const TableEntryVal* new_entry_val, if ( ! handle ) return; - // we wither get passed the raw index_val - or a ListVal with exactly one element. + // we either get passed the raw index_val - or a ListVal with exactly one element. // Since broker does not support ListVals, we have to unoll this in the second case. const Val* index_val; if ( index->GetType()->Tag() == zeek::TYPE_LIST ) @@ -2214,6 +2224,10 @@ void TableVal::SendToStore(const Val* index, const TableEntryVal* new_entry_val, IntrusivePtr TableVal::Remove(const Val& index, bool broker_forward) { auto k = MakeHashKey(index); + // this is totally cheating around the fact that we need a Intrusive pointer. + IntrusivePtr changefunc_val; + if ( change_func ) + changefunc_val = RecreateIndex(*(k.get())); TableEntryVal* v = k ? AsNonConstTable()->RemoveEntry(k.get()) : nullptr; IntrusivePtr va; @@ -2230,7 +2244,7 @@ IntrusivePtr TableVal::Remove(const Val& index, bool broker_forward) if ( broker_forward && ! broker_store.empty() ) SendToStore(&index, nullptr, ELEMENT_REMOVED); if ( change_func ) - CallChangeFunc(&index, va, ELEMENT_REMOVED); + CallChangeFunc(changefunc_val, va, ELEMENT_REMOVED); return va; } @@ -2255,13 +2269,13 @@ IntrusivePtr TableVal::Remove(const HashKey& k) Modified(); - if ( ( change_func && va ) || ( ! broker_store.empty() ) ) + if ( va && ( change_func || ! broker_store.empty() ) ) { auto index = table_hash->RecoverVals(k); if ( ! broker_store.empty() ) SendToStore(index.get(), nullptr, ELEMENT_REMOVED); if ( change_func && va ) - CallChangeFunc(index.get(), va, ELEMENT_REMOVED); + CallChangeFunc(index, va, ELEMENT_REMOVED); } return va; @@ -2565,7 +2579,7 @@ void TableVal::DoExpire(double t) { if ( ! idx ) idx = RecreateIndex(*k); - CallChangeFunc(idx.get(), v->GetVal(), ELEMENT_EXPIRED); + CallChangeFunc(idx, v->GetVal(), ELEMENT_EXPIRED); } delete v; diff --git a/src/Val.h b/src/Val.h index bbd1fb7e2d..019a3d6cba 100644 --- a/src/Val.h +++ b/src/Val.h @@ -1070,8 +1070,8 @@ protected: // Enum for the different kinds of changes an &on_change handler can see enum OnChangeType { ELEMENT_NEW, ELEMENT_CHANGED, ELEMENT_REMOVED, ELEMENT_EXPIRED }; - // Calls &change_func. Does not take ownership of values. (Refs if needed). - void CallChangeFunc(const Val* index, const IntrusivePtr& old_value, + // Calls &change_func. + void CallChangeFunc(const IntrusivePtr& index, const IntrusivePtr& old_value, OnChangeType tpe); // Sends data on to backing Broker Store diff --git a/testing/btest/broker/store/brokerstore-attr-expire.zeek b/testing/btest/broker/store/brokerstore-attr-expire.zeek index 32ca097de7..0b12e4df96 100644 --- a/testing/btest/broker/store/brokerstore-attr-expire.zeek +++ b/testing/btest/broker/store/brokerstore-attr-expire.zeek @@ -29,22 +29,22 @@ type testrec: record { c: set[string]; }; -function expire_t(tbl: any, idx: string): interval +function change_t(tbl: any, tpe: TableChange, idx: string, idxb: count) { - print fmt("Expiring t: %s", idx); - return 0sec; + if ( tpe == TABLE_ELEMENT_EXPIRED ) + print fmt("Expiring t: %s", idx); } -function expire_s(tbl: any, idx: string): interval +function change_s(tbl: any, tpe: TableChange, idx: string, idbx: count) { - print fmt("Expiring s: %s", idx); - return 0sec; + if ( tpe == TABLE_ELEMENT_EXPIRED ) + print fmt("Expiring s: %s", idx); } -function expire_r(tbl: any, idx: string): interval +function change_r(tbl: any, tpe: TableChange, idx: string, idxb: testrec) { - print fmt("Expiring r: %s", idx); - return 0sec; + if ( tpe == TABLE_ELEMENT_EXPIRED ) + print fmt("Expiring r: %s", idx); } function print_keys() @@ -60,9 +60,9 @@ function print_keys() } } -global t: table[string] of count &broker_store="table" &create_expire=4sec &expire_func=expire_t; -global s: table[string] of count &broker_store="set" &write_expire=3sec &expire_func=expire_s; -global r: table[string] of testrec &broker_store="rec" &read_expire=5sec &expire_func=expire_r; +global t: table[string] of count &broker_store="table" &create_expire=4sec &on_change=change_t; +global s: table[string] of count &broker_store="set" &write_expire=3sec &on_change=change_s; +global r: table[string] of testrec &broker_store="rec" &write_expire=5sec &on_change=change_r; event zeek_init() { @@ -134,27 +134,27 @@ type testrec: record { c: set[string]; }; -function expire_t(tbl: any, idx: string): interval +function change_t(tbl: any, tpe: TableChange, idx: string, idxb: count) { - print fmt("Expiring t: %s", idx); - return 0sec; + if ( tpe == TABLE_ELEMENT_EXPIRED ) + print fmt("Expiring t: %s", idx); } -function expire_s(tbl: any, idx: string): interval +function change_s(tbl: any, tpe: TableChange, idx: string, idbx: count) { - print fmt("Expiring s: %s", idx); - return 0sec; + if ( tpe == TABLE_ELEMENT_EXPIRED ) + print fmt("Expiring s: %s", idx); } -function expire_r(tbl: any, idx: string): interval +function change_r(tbl: any, tpe: TableChange, idx: string, idxb: testrec) { - print fmt("Expiring r: %s", idx); - return 0sec; + if ( tpe == TABLE_ELEMENT_EXPIRED ) + print fmt("Expiring r: %s", idx); } -global t: table[string] of count &broker_store="table" &create_expire=4sec &expire_func=expire_t; -global s: table[string] of count &broker_store="set" &write_expire=3sec &expire_func=expire_s; -global r: table[string] of testrec &broker_store="rec" &read_expire=5sec &expire_func=expire_r; +global t: table[string] of count &broker_store="table" &create_expire=4sec &on_change=change_t; +global s: table[string] of count &broker_store="set" &write_expire=3sec &on_change=change_s; +global r: table[string] of testrec &broker_store="rec" &write_expire=5sec &on_change=change_r; event zeek_init() { From b9fe79c69773f846a5f9eb0427a0cf17bdca9e74 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Fri, 26 Jun 2020 16:10:15 -0700 Subject: [PATCH 11/26] BrokerStore<->Zeek tables: load persistent tables on startup. This currently only handles the most basic case, and is not thoroughly tested. When initializing a master store, we now check if there already is data in it. If yes, we load it directly into the zeek table when the store is created. We assume that this is happening at Zeek startup - and are supremely evil and just load it synchronously. Which could block execution for a bit for larger stores. That being said - this might sidestep other issues that would arise when doing this async (like scripts already inserting data). Next step: check if this approach also works for clones. --- src/broker/Manager.cc | 61 ++++++++++++-- src/broker/Manager.h | 4 + .../output1 | 20 +++++ .../output2 | 20 +++++ .../store/brokerstore-attr-persistence.zeek | 80 +++++++++++++++++++ 5 files changed, 180 insertions(+), 5 deletions(-) create mode 100644 testing/btest/Baseline/broker.store.brokerstore-attr-persistence/output1 create mode 100644 testing/btest/Baseline/broker.store.brokerstore-attr-persistence/output2 create mode 100644 testing/btest/broker/store/brokerstore-attr-persistence.zeek diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index 18403b9f9c..b9c95125e1 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -1608,15 +1608,66 @@ StoreHandleVal* Manager::MakeMaster(const string& name, broker::backend type, iosource_mgr->RegisterFd(handle->proxy.mailbox().descriptor(), this); CheckForwarding(name); - if ( bstate->endpoint.use_real_time() ) - return handle; + if ( ! bstate->endpoint.use_real_time() ) + // Wait for master to become available/responsive. + // Possibly avoids timeouts in scripts during unit tests. + handle->store.exists(""); + + BrokerStoreToZeekTable(name, handle); - // Wait for master to become available/responsive. - // Possibly avoids timeouts in scripts during unit tests. - handle->store.exists(""); return handle; } +void Manager::BrokerStoreToZeekTable(const std::string& name, const StoreHandleVal* handle) + { + // consider if it might be wise to disable &on_change while filling the table + if ( ! handle->forward_to ) + return; + + auto keys = handle->store.keys(); + if ( ! keys ) + return; + + auto set = caf::get_if(&(keys->get_data())); + auto table = handle->forward_to; + const auto& its = table->GetType()->AsTableType()->IndexTypes(); + bool is_set = table->GetType()->IsSet(); + assert( its.size() == 1 ); + for ( const auto key : *set ) + { + auto zeek_key = data_to_val(key, its[0].get()); + if ( ! zeek_key ) + { + reporter->Error("Failed to convert key %s while importing broker store to table for store %s. Aborting import.", to_string(key).c_str(), name.c_str()); + // just abort - this probably means the types are incompatible + return; + } + + if ( is_set ) + { + table->Assign(zeek_key, nullptr, false); + continue; + } + + auto value = handle->store.get(key); + if ( ! value ) + { + reporter->Error("Failed to load value for key %s while importing broker store %s to table", to_string(key).c_str(), name.c_str()); + continue; + } + + auto zeek_value = data_to_val(*value, table->GetType()->Yield().get()); + if ( ! zeek_value ) + { + reporter->Error("Could not convert %s to table value while trying to import broker store %s. Aborting import.", to_string(value).c_str(), name.c_str()); + return; + } + + table->Assign(zeek_key, zeek_value, false); + } + return; + } + StoreHandleVal* Manager::MakeClone(const string& name, double resync_interval, double stale_interval, double mutation_buffer_interval) diff --git a/src/broker/Manager.h b/src/broker/Manager.h index 6cfcc05a9b..9ccefde4b0 100644 --- a/src/broker/Manager.h +++ b/src/broker/Manager.h @@ -355,7 +355,11 @@ private: void ProcessError(broker::error err); void ProcessStoreResponse(StoreHandleVal*, broker::store::response response); void FlushPendingQueries(); + // Check if a broker store is associated to a table on the Zeek side. void CheckForwarding(const std::string& name); + // Send the content of a broker store to the backing table. This is typically used + // when a master/clone is created. + void BrokerStoreToZeekTable(const std::string& name, const StoreHandleVal* handle); void Error(const char* format, ...) __attribute__((format (printf, 2, 3))); diff --git a/testing/btest/Baseline/broker.store.brokerstore-attr-persistence/output1 b/testing/btest/Baseline/broker.store.brokerstore-attr-persistence/output1 new file mode 100644 index 0000000000..1710727ce2 --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-attr-persistence/output1 @@ -0,0 +1,20 @@ +{ +[b] = 3, +[whatever] = 5, +[a] = 5 +} +{ +I am really a set!, +Believe me - I am a set, +I am a set! +} +{ +[b] = [a=2, b=d, c={ +elem1, +elem2 +}], +[a] = [a=1, b=c, c={ +elem1, +elem2 +}] +} diff --git a/testing/btest/Baseline/broker.store.brokerstore-attr-persistence/output2 b/testing/btest/Baseline/broker.store.brokerstore-attr-persistence/output2 new file mode 100644 index 0000000000..1710727ce2 --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-attr-persistence/output2 @@ -0,0 +1,20 @@ +{ +[b] = 3, +[whatever] = 5, +[a] = 5 +} +{ +I am really a set!, +Believe me - I am a set, +I am a set! +} +{ +[b] = [a=2, b=d, c={ +elem1, +elem2 +}], +[a] = [a=1, b=c, c={ +elem1, +elem2 +}] +} diff --git a/testing/btest/broker/store/brokerstore-attr-persistence.zeek b/testing/btest/broker/store/brokerstore-attr-persistence.zeek new file mode 100644 index 0000000000..5004b12b91 --- /dev/null +++ b/testing/btest/broker/store/brokerstore-attr-persistence.zeek @@ -0,0 +1,80 @@ +# @TEST-PORT: BROKER_PORT + +# @TEST-EXEC: zeek -B broker -b one.zeek > output1 +# @TEST-EXEC: zeek -B broker -b two.zeek > output2 +# @TEST-EXEC: btest-diff output1 +# @TEST-EXEC: btest-diff output2 +# @TEST-EXEC: diff output1 output2 + +# the first test writes out the sqlite files... + +@TEST-START-FILE one.zeek + +module TestModule; + +global tablestore: opaque of Broker::Store; +global setstore: opaque of Broker::Store; +global recordstore: opaque of Broker::Store; + +type testrec: record { + a: count; + b: string; + c: set[string]; +}; + +global t: table[string] of count &broker_store="table"; +global s: set[string] &broker_store="set"; +global r: table[string] of testrec &broker_store="rec"; + +event zeek_init() + { + tablestore = Broker::create_master("table", Broker::SQLITE); + setstore = Broker::create_master("set", Broker::SQLITE); + recordstore = Broker::create_master("rec", Broker::SQLITE); + t["a"] = 5; + t["b"] = 3; + t["c"] = 4; + t["whatever"] = 5; + delete t["c"]; + add s["I am a set!"]; + add s["I am really a set!"]; + add s["Believe me - I am a set"]; + r["a"] = testrec($a=1, $b="b", $c=set("elem1", "elem2")); + r["a"] = testrec($a=1, $b="c", $c=set("elem1", "elem2")); + r["b"] = testrec($a=2, $b="d", $c=set("elem1", "elem2")); + print t; + print s; + print r; + } + +@TEST-END-FILE +@TEST-START-FILE two.zeek + +# the second one reads them in again + +module TestModule; + +global tablestore: opaque of Broker::Store; +global setstore: opaque of Broker::Store; +global recordstore: opaque of Broker::Store; + +type testrec: record { + a: count; + b: string; + c: set[string]; +}; + +global t: table[string] of count &broker_store="table"; +global s: set[string] &broker_store="set"; +global r: table[string] of testrec &broker_store="rec"; + +event zeek_init() + { + tablestore = Broker::create_master("table", Broker::SQLITE); + setstore = Broker::create_master("set", Broker::SQLITE); + recordstore = Broker::create_master("rec", Broker::SQLITE); + print t; + print s; + print r; + } +@TEST-END-FILE From 43d228975420c9bfda6ad9b7d907eb6e82beedc8 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Mon, 29 Jun 2020 15:05:39 -0700 Subject: [PATCH 12/26] BrokerStore<->Zeek tables: test for clones synchronizing to a master When a clone attaches to a master, it just gets the diffs sent as events. Which is neat because it means that we pretty much don't need any extra code to handle this. --- .../output1 | 20 +++ .../output2 | 20 +++ .../output3 | 20 +++ .../brokerstore-attr-persistence-clone.zeek | 141 ++++++++++++++++++ 4 files changed, 201 insertions(+) create mode 100644 testing/btest/Baseline/broker.store.brokerstore-attr-persistence-clone/output1 create mode 100644 testing/btest/Baseline/broker.store.brokerstore-attr-persistence-clone/output2 create mode 100644 testing/btest/Baseline/broker.store.brokerstore-attr-persistence-clone/output3 create mode 100644 testing/btest/broker/store/brokerstore-attr-persistence-clone.zeek diff --git a/testing/btest/Baseline/broker.store.brokerstore-attr-persistence-clone/output1 b/testing/btest/Baseline/broker.store.brokerstore-attr-persistence-clone/output1 new file mode 100644 index 0000000000..1710727ce2 --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-attr-persistence-clone/output1 @@ -0,0 +1,20 @@ +{ +[b] = 3, +[whatever] = 5, +[a] = 5 +} +{ +I am really a set!, +Believe me - I am a set, +I am a set! +} +{ +[b] = [a=2, b=d, c={ +elem1, +elem2 +}], +[a] = [a=1, b=c, c={ +elem1, +elem2 +}] +} diff --git a/testing/btest/Baseline/broker.store.brokerstore-attr-persistence-clone/output2 b/testing/btest/Baseline/broker.store.brokerstore-attr-persistence-clone/output2 new file mode 100644 index 0000000000..1710727ce2 --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-attr-persistence-clone/output2 @@ -0,0 +1,20 @@ +{ +[b] = 3, +[whatever] = 5, +[a] = 5 +} +{ +I am really a set!, +Believe me - I am a set, +I am a set! +} +{ +[b] = [a=2, b=d, c={ +elem1, +elem2 +}], +[a] = [a=1, b=c, c={ +elem1, +elem2 +}] +} diff --git a/testing/btest/Baseline/broker.store.brokerstore-attr-persistence-clone/output3 b/testing/btest/Baseline/broker.store.brokerstore-attr-persistence-clone/output3 new file mode 100644 index 0000000000..1710727ce2 --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-attr-persistence-clone/output3 @@ -0,0 +1,20 @@ +{ +[b] = 3, +[whatever] = 5, +[a] = 5 +} +{ +I am really a set!, +Believe me - I am a set, +I am a set! +} +{ +[b] = [a=2, b=d, c={ +elem1, +elem2 +}], +[a] = [a=1, b=c, c={ +elem1, +elem2 +}] +} diff --git a/testing/btest/broker/store/brokerstore-attr-persistence-clone.zeek b/testing/btest/broker/store/brokerstore-attr-persistence-clone.zeek new file mode 100644 index 0000000000..5f6b9ffe87 --- /dev/null +++ b/testing/btest/broker/store/brokerstore-attr-persistence-clone.zeek @@ -0,0 +1,141 @@ +# @TEST-PORT: BROKER_PORT + +# @TEST-EXEC: zeek -B broker -b one.zeek > output1 +# @TEST-EXEC: btest-bg-run master "cp ../*.sqlite . && zeek -B broker -b ../two.zeek >../output2" +# @TEST-EXEC: btest-bg-run clone "zeek -B broker -b ../three.zeek >../output3" +# @TEST-EXEC: btest-bg-wait 15 + +# @TEST-EXEC: btest-diff output1 +# @TEST-EXEC: btest-diff output2 +# @TEST-EXEC: btest-diff output3 +# @TEST-EXEC: diff output1 output2 +# @TEST-EXEC: diff output2 output3 + +# the first test writes out the sqlite files... + +@TEST-START-FILE one.zeek + +module TestModule; + +global tablestore: opaque of Broker::Store; +global setstore: opaque of Broker::Store; +global recordstore: opaque of Broker::Store; + +type testrec: record { + a: count; + b: string; + c: set[string]; +}; + +global t: table[string] of count &broker_store="table"; +global s: set[string] &broker_store="set"; +global r: table[string] of testrec &broker_store="rec"; + +event zeek_init() + { + tablestore = Broker::create_master("table", Broker::SQLITE); + setstore = Broker::create_master("set", Broker::SQLITE); + recordstore = Broker::create_master("rec", Broker::SQLITE); + t["a"] = 5; + t["b"] = 3; + t["c"] = 4; + t["whatever"] = 5; + delete t["c"]; + add s["I am a set!"]; + add s["I am really a set!"]; + add s["Believe me - I am a set"]; + r["a"] = testrec($a=1, $b="b", $c=set("elem1", "elem2")); + r["a"] = testrec($a=1, $b="c", $c=set("elem1", "elem2")); + r["b"] = testrec($a=2, $b="d", $c=set("elem1", "elem2")); + print t; + print s; + print r; + } + +@TEST-END-FILE +@TEST-START-FILE two.zeek + +# read in again - and serve to clones + +redef exit_only_after_terminate = T; + +module TestModule; + +global tablestore: opaque of Broker::Store; +global setstore: opaque of Broker::Store; +global recordstore: opaque of Broker::Store; + +type testrec: record { + a: count; + b: string; + c: set[string]; +}; + +global t: table[string] of count &broker_store="table"; +global s: set[string] &broker_store="set"; +global r: table[string] of testrec &broker_store="rec"; + +event zeek_init() + { + Broker::listen("127.0.0.1", to_port(getenv("BROKER_PORT"))); + tablestore = Broker::create_master("table", Broker::SQLITE); + setstore = Broker::create_master("set", Broker::SQLITE); + recordstore = Broker::create_master("rec", Broker::SQLITE); + print t; + print s; + print r; + } + +event Broker::peer_lost(endpoint: Broker::EndpointInfo, msg: string) + { + terminate(); + } + +@TEST-END-FILE + +@TEST-START-FILE three.zeek + +# get copy from master + +redef exit_only_after_terminate = T; + +module TestModule; + +global tablestore: opaque of Broker::Store; +global setstore: opaque of Broker::Store; +global recordstore: opaque of Broker::Store; + +type testrec: record { + a: count; + b: string; + c: set[string]; +}; + + +global t: table[string] of count &broker_store="table"; +global s: set[string] &broker_store="set"; +global r: table[string] of testrec &broker_store="rec"; + +event zeek_init() + { + Broker::peer("127.0.0.1", to_port(getenv("BROKER_PORT"))); + } + +event print_me() + { + print t; + print s; + print r; + terminate(); + } + +event Broker::peer_added(endpoint: Broker::EndpointInfo, msg: string) + { + tablestore = Broker::create_clone("table"); + setstore = Broker::create_clone("set"); + recordstore = Broker::create_clone("rec"); + schedule 2sec { print_me() }; + } + + +@TEST-END-FILE From 318a72c3031d490b841c52b3737e13d3e0ba7fec Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Tue, 30 Jun 2020 16:33:52 -0700 Subject: [PATCH 13/26] BrokerStore<->Zeek table - introdude &backend attribute The &backend attribute allows for a much more convenient way of interacting with brokerstores. One does not need to create a broker store anymore - instead all of this is done internally. The current state of this partially works. This should work fine for persistence - but clones are currently not yet correctly attached. --- scripts/base/frameworks/broker/store.zeek | 4 + scripts/base/frameworks/cluster/main.zeek | 4 + src/Attr.cc | 40 +++++++- src/Attr.h | 2 + src/Val.h | 2 + src/broker/Manager.cc | 31 ++++++ src/broker/Manager.h | 3 + src/parse.y | 6 +- src/scan.l | 1 + .../store/brokerstore-backend-simple.zeek | 97 +++++++++++++++++++ 10 files changed, 187 insertions(+), 3 deletions(-) create mode 100644 testing/btest/broker/store/brokerstore-backend-simple.zeek diff --git a/scripts/base/frameworks/broker/store.zeek b/scripts/base/frameworks/broker/store.zeek index 50559c4522..2456ccbf2f 100644 --- a/scripts/base/frameworks/broker/store.zeek +++ b/scripts/base/frameworks/broker/store.zeek @@ -25,6 +25,10 @@ export { ## A negative/zero value indicates to never buffer commands. const default_clone_mutation_buffer_interval = 2min &redef; + const auto_store_master = T &redef; + + const auto_store_db_directory = "." &redef; + ## Whether a data store query could be completed or not. type QueryStatus: enum { SUCCESS, diff --git a/scripts/base/frameworks/cluster/main.zeek b/scripts/base/frameworks/cluster/main.zeek index 1f39a7f7cf..28c4630d7a 100644 --- a/scripts/base/frameworks/cluster/main.zeek +++ b/scripts/base/frameworks/cluster/main.zeek @@ -249,6 +249,10 @@ export { global nodeid_topic: function(id: string): string; } +@if ( Cluster::is_enabled() && Cluster::local_node_type() != Cluster::MANAGER ) ) +redef Broker::store_master = T; +@endif + global active_worker_ids: set[string] = set(); type NamedNode: record { diff --git a/src/Attr.cc b/src/Attr.cc index 5b56fe1a07..1bcd8827ae 100644 --- a/src/Attr.cc +++ b/src/Attr.cc @@ -19,7 +19,7 @@ const char* attr_name(attr_tag t) "&read_expire", "&write_expire", "&create_expire", "&raw_output", "&priority", "&group", "&log", "&error_handler", "&type_column", - "(&tracked)", "&on_change", "&broker_store", + "(&tracked)", "&on_change", "&broker_store", "&backend", "&deprecated", }; @@ -616,6 +616,43 @@ void Attributes::CheckAttr(Attr* a) } break; + case ATTR_BACKEND: + { + if ( ! global_var || type->Tag() != TYPE_TABLE ) + { + Error("&broker_store only applicable to global sets/tables"); + break; + } + + // cannot do better equality check - the broker types are not actually existing yet when we + // are here. We will do that later - before actually attaching to a broker store + if ( a->GetExpr()->GetType()->Tag() != TYPE_ENUM ) + { + Error("&broker_store must take an enum argument"); + break; + } + + // Temporary since Broker does not support ListVals - and we cannot easily convert to set/vector + if ( type->AsTableType()->IndexTypes().size() != 1 ) + { + Error("&backend only supports one-element set/table indexes"); + } + + if ( Find(ATTR_EXPIRE_FUNC ) ) + { + Error("&backend and &expire_func cannot be used simultaneously"); + } + if ( Find(ATTR_EXPIRE_READ) ) + { + Error("&backend and &read_expire cannot be used simultaneously"); + } + if ( Find(ATTR_BROKER_STORE) ) + { + Error("&backend and &broker_store cannot be used simultaneously"); + } + + break; + } case ATTR_BROKER_STORE: { if ( type->Tag() != TYPE_TABLE ) @@ -635,6 +672,7 @@ void Attributes::CheckAttr(Attr* a) { Error("&broker_store only supports one-element set/table indexes"); } + if ( Find(ATTR_EXPIRE_FUNC ) ) { Error("&broker_store and &expire_func cannot be used simultaneously"); diff --git a/src/Attr.h b/src/Attr.h index a1a2677981..be4cc7c605 100644 --- a/src/Attr.h +++ b/src/Attr.h @@ -33,6 +33,7 @@ enum [[deprecated("Remove in v4.1. Use zeek::detail::attr_tag instead.")]] attr_ ATTR_TRACKED, // hidden attribute, tracked by NotifierRegistry ATTR_ON_CHANGE, // for table change tracking ATTR_BROKER_STORE, // for broker-store backed tables + ATTR_BACKEND, // for broker-store backed tabled ATTR_DEPRECATED, NUM_ATTRS // this item should always be last }; @@ -58,6 +59,7 @@ enum attr_tag { ATTR_TRACKED, // hidden attribute, tracked by NotifierRegistry ATTR_ON_CHANGE, // for table change tracking ATTR_BROKER_STORE, // for broker-store backed tables + ATTR_BACKEND, // for broker-store backed tabled ATTR_DEPRECATED, NUM_ATTRS // this item should always be last }; diff --git a/src/Val.h b/src/Val.h index d3f3e18f1a..be87a59ee3 100644 --- a/src/Val.h +++ b/src/Val.h @@ -1030,6 +1030,8 @@ public: // on zeek::RecordTypes. static void DoneParsing(); + void SetBrokerStore(const std::string& store) { broker_store = store; } + protected: void Init(IntrusivePtr t); diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index b9c95125e1..435f2e921a 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -151,6 +151,8 @@ void Manager::InitPostScript() log_topic_func = get_option("Broker::log_topic")->AsFunc(); log_id_type = zeek::id::find_type("Log::ID")->AsEnumType(); writer_id_type = zeek::id::find_type("Log::Writer")->AsEnumType(); + zeek_table_manager = get_option("Broker::auto_store_master")->AsBool(); + zeek_table_db_directory = get_option("Broker::auto_store_db_directory")->AsString()->CheckString(); opaque_of_data_type = make_intrusive("Broker::Data"); opaque_of_set_iterator = make_intrusive("Broker::SetIterator"); @@ -214,6 +216,35 @@ void Manager::InitPostScript() reporter->FatalError("Failed to register broker status subscriber with iosource_mgr"); bstate->subscriber.add_topic(broker::topics::store_events, true); + + InitializeBrokerStoreForwarding(); + } + +void Manager::InitializeBrokerStoreForwarding() + { + const auto& globals = global_scope()->Vars(); + + for ( const auto& global : globals ) + { + auto& id = global.second; + if ( id->HasVal() && id->GetAttr(zeek::detail::ATTR_BACKEND) ) + { + const auto& attr = id->GetAttr(zeek::detail::ATTR_BACKEND); + auto e = static_cast(attr->GetExpr()->Eval(nullptr)->AsEnum()); + auto storename = std::string("___sync_store_") + global.first; + id->GetVal()->AsTableVal()->SetBrokerStore(storename); + AddForwardedStore(storename, {NewRef{}, id->GetVal()->AsTableVal()}); + + auto backend = bro_broker::to_backend_type(e); + + if ( zeek_table_manager ) + MakeMaster(storename, backend, broker::backend_options{}); + else + { + MakeClone(storename); + } + } + } } void Manager::Terminate() diff --git a/src/broker/Manager.h b/src/broker/Manager.h index 9ccefde4b0..201040aaaf 100644 --- a/src/broker/Manager.h +++ b/src/broker/Manager.h @@ -355,6 +355,7 @@ private: void ProcessError(broker::error err); void ProcessStoreResponse(StoreHandleVal*, broker::store::response response); void FlushPendingQueries(); + void InitializeBrokerStoreForwarding(); // Check if a broker store is associated to a table on the Zeek side. void CheckForwarding(const std::string& name); // Send the content of a broker store to the backing table. This is typically used @@ -411,6 +412,8 @@ private: IntrusivePtr vector_of_data_type; zeek::EnumType* log_id_type; zeek::EnumType* writer_id_type; + bool zeek_table_manager = false; + std::string zeek_table_db_directory; static int script_scope; }; diff --git a/src/parse.y b/src/parse.y index f5576f7995..d8d7abcf95 100644 --- a/src/parse.y +++ b/src/parse.y @@ -5,7 +5,7 @@ // Switching parser table type fixes ambiguity problems. %define lr.type ielr -%expect 117 +%expect 123 %token TOK_ADD TOK_ADD_TO TOK_ADDR TOK_ANY %token TOK_ATENDIF TOK_ATELSE TOK_ATIF TOK_ATIFDEF TOK_ATIFNDEF @@ -24,7 +24,7 @@ %token TOK_ATTR_ADD_FUNC TOK_ATTR_DEFAULT TOK_ATTR_OPTIONAL TOK_ATTR_REDEF %token TOK_ATTR_DEL_FUNC TOK_ATTR_EXPIRE_FUNC %token TOK_ATTR_EXPIRE_CREATE TOK_ATTR_EXPIRE_READ TOK_ATTR_EXPIRE_WRITE -%token TOK_ATTR_RAW_OUTPUT TOK_ATTR_ON_CHANGE TOK_ATTR_BROKER_STORE +%token TOK_ATTR_RAW_OUTPUT TOK_ATTR_ON_CHANGE TOK_ATTR_BROKER_STORE TOK_ATTR_BACKEND %token TOK_ATTR_PRIORITY TOK_ATTR_LOG TOK_ATTR_ERROR_HANDLER %token TOK_ATTR_TYPE_COLUMN TOK_ATTR_DEPRECATED @@ -1367,6 +1367,8 @@ attr: { $$ = new zeek::detail::Attr(zeek::detail::ATTR_ON_CHANGE, {AdoptRef{}, $3}); } | TOK_ATTR_BROKER_STORE '=' expr { $$ = new zeek::detail::Attr(zeek::detail::ATTR_BROKER_STORE, {AdoptRef{}, $3}); } + | TOK_ATTR_BACKEND '=' expr + { $$ = new zeek::detail::Attr(zeek::detail::ATTR_BACKEND, {AdoptRef{}, $3}); } | TOK_ATTR_EXPIRE_FUNC '=' expr { $$ = new zeek::detail::Attr(zeek::detail::ATTR_EXPIRE_FUNC, {AdoptRef{}, $3}); } | TOK_ATTR_EXPIRE_CREATE '=' expr diff --git a/src/scan.l b/src/scan.l index bcaa084663..b2a716229a 100644 --- a/src/scan.l +++ b/src/scan.l @@ -313,6 +313,7 @@ when return TOK_WHEN; &write_expire return TOK_ATTR_EXPIRE_WRITE; &on_change return TOK_ATTR_ON_CHANGE; &broker_store return TOK_ATTR_BROKER_STORE; +&backend return TOK_ATTR_BACKEND; @deprecated.* { auto num_files = file_stack.length(); diff --git a/testing/btest/broker/store/brokerstore-backend-simple.zeek b/testing/btest/broker/store/brokerstore-backend-simple.zeek new file mode 100644 index 0000000000..1382fdc7e3 --- /dev/null +++ b/testing/btest/broker/store/brokerstore-backend-simple.zeek @@ -0,0 +1,97 @@ +# @TEST-PORT: BROKER_PORT + +# @TEST-EXEC: btest-bg-run master "zeek -B broker -b ../master.zeek >../master.out" +# @TEST-EXEC: btest-bg-run clone "zeek -B broker -b ../clone.zeek >../clone.out" +# @TEST-EXEC: btest-bg-wait 15 +# +# @TEST-EXEC: btest-diff clone.out + +@TEST-START-FILE master.zeek +redef exit_only_after_terminate = T; + +module TestModule; + +type testrec: record { + a: count; + b: string; + c: set[string]; +}; + +global t: table[string] of count &backend=Broker::MEMORY; +global s: set[string] &backend=Broker::MEMORY; +global r: table[string] of testrec &backend=Broker::MEMORY; + +event zeek_init() + { + Broker::listen("127.0.0.1", to_port(getenv("BROKER_PORT"))); + } + +event insert_stuff() + { + print "Inserting stuff"; + t["a"] = 5; + delete t["a"]; + add s["hi"]; + t["a"] = 2; + t["a"] = 3; + t["b"] = 3; + t["c"] = 4; + t["whatever"] = 5; + delete t["c"]; + r["a"] = testrec($a=1, $b="b", $c=set("elem1", "elem2")); + r["a"] = testrec($a=1, $b="c", $c=set("elem1", "elem2")); + r["b"] = testrec($a=2, $b="d", $c=set("elem1", "elem2")); + print t; + print s; + print r; + } + +event Broker::peer_added(endpoint: Broker::EndpointInfo, msg: string) + { + print "Peer added ", endpoint; + schedule 3secs { insert_stuff() }; + } + +event Broker::peer_lost(endpoint: Broker::EndpointInfo, msg: string) + { + terminate(); + } + +@TEST-END-FILE + +@TEST-START-FILE clone.zeek +redef exit_only_after_terminate = T; +redef Broker::auto_store_master = F; + +module TestModule; + +type testrec: record { + a: count; + b: string; + c: set[string]; +}; + +global t: table[string] of count &backend=Broker::MEMORY; +global s: set[string] &backend=Broker::MEMORY; +global r: table[string] of testrec &backend=Broker::MEMORY; + + +event zeek_init() + { + Broker::peer("127.0.0.1", to_port(getenv("BROKER_PORT"))); + } + +event dump_tables() + { + print t; + print s; + print r; + terminate(); + } + +event Broker::peer_added(endpoint: Broker::EndpointInfo, msg: string) + { + print "Peer added"; + schedule 5secs { dump_tables() }; + } +@TEST-END-FILE From a220b027221faa310341a23ee6d6ef9607e07011 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Wed, 1 Jul 2020 16:38:10 -0700 Subject: [PATCH 14/26] BrokerStore<->Zeek tables: &backend works for in-memory stores. Currently this requires using this with a normal cluster - or sending messages by yourself. It, in principle, should also work with SQLITE - but that is a bit nonsensical without being able to change the storage location. --- scripts/base/frameworks/cluster/__load__.zeek | 2 + .../frameworks/cluster/broker-stores.zeek | 46 +++++++++++++++++ scripts/base/frameworks/cluster/main.zeek | 4 -- scripts/base/init-bare.zeek | 1 + src/broker/Manager.cc | 10 ++-- src/zeek.bif | 3 +- .../clone.out | 18 +++++++ .../master.out | 18 +++++++ .../store/brokerstore-backend-simple.zeek | 49 ++++++++++--------- 9 files changed, 118 insertions(+), 33 deletions(-) create mode 100644 scripts/base/frameworks/cluster/broker-stores.zeek create mode 100644 testing/btest/Baseline/broker.store.brokerstore-backend-simple/clone.out create mode 100644 testing/btest/Baseline/broker.store.brokerstore-backend-simple/master.out diff --git a/scripts/base/frameworks/cluster/__load__.zeek b/scripts/base/frameworks/cluster/__load__.zeek index a04d6744d2..47918e7d0d 100644 --- a/scripts/base/frameworks/cluster/__load__.zeek +++ b/scripts/base/frameworks/cluster/__load__.zeek @@ -49,5 +49,7 @@ redef Broker::log_topic = Cluster::rr_log_topic; @load ./nodes/worker @endif +@load ./broker-stores.zeek + @endif @endif diff --git a/scripts/base/frameworks/cluster/broker-stores.zeek b/scripts/base/frameworks/cluster/broker-stores.zeek new file mode 100644 index 0000000000..11155e5387 --- /dev/null +++ b/scripts/base/frameworks/cluster/broker-stores.zeek @@ -0,0 +1,46 @@ + +module Broker; + +export { + global announce_masters: event(masters: set[string]); +} + +@if ( Cluster::is_enabled() && Cluster::local_node_type() != Cluster::MANAGER ) +redef Broker::auto_store_master = F; +@endif + +@if ( Broker::auto_store_master ) + +global broker_backed_ids: set[string]; + +event zeek_init() + { + local globals = global_ids(); + for ( id in globals ) + { + if ( globals[id]$broker_backend ) + add broker_backed_ids[id]; + } + } + +event Broker::peer_added(endpoint: Broker::EndpointInfo, msg: string) &priority=1 + { + if ( ! Cluster::is_enabled() ) + return; + + local e = Broker::make_event(Broker::announce_masters, broker_backed_ids); + Broker::publish(Cluster::nodeid_topic(endpoint$id), e); + } + +@else + +event Broker::announce_masters(masters: set[string]) + { + for ( i in masters ) + { + local name = "___sync_store_" + i; + Broker::create_clone(name); + } + } + +@endif diff --git a/scripts/base/frameworks/cluster/main.zeek b/scripts/base/frameworks/cluster/main.zeek index 28c4630d7a..1f39a7f7cf 100644 --- a/scripts/base/frameworks/cluster/main.zeek +++ b/scripts/base/frameworks/cluster/main.zeek @@ -249,10 +249,6 @@ export { global nodeid_topic: function(id: string): string; } -@if ( Cluster::is_enabled() && Cluster::local_node_type() != Cluster::MANAGER ) ) -redef Broker::store_master = T; -@endif - global active_worker_ids: set[string] = set(); type NamedNode: record { diff --git a/scripts/base/init-bare.zeek b/scripts/base/init-bare.zeek index a02a676df9..d65b97b116 100644 --- a/scripts/base/init-bare.zeek +++ b/scripts/base/init-bare.zeek @@ -734,6 +734,7 @@ type script_id: record { enum_constant: bool; ##< True if the identifier is an enum value. option_value: bool; ##< True if the identifier is an option. redefinable: bool; ##< True if the identifier is declared with the :zeek:attr:`&redef` attribute. + broker_backend: bool; ##< True if the identifier has a broker backend defined using the :zeek:attr:`&backend` attribute. value: any &optional; ##< The current value of the identifier. }; diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index 435f2e921a..b897083a40 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -235,13 +235,15 @@ void Manager::InitializeBrokerStoreForwarding() id->GetVal()->AsTableVal()->SetBrokerStore(storename); AddForwardedStore(storename, {NewRef{}, id->GetVal()->AsTableVal()}); - auto backend = bro_broker::to_backend_type(e); + // we only create masters here. For clones, we do all the work of setting up + // the forwarding - but we do not try to initialize the clone. We can only initialize + // the clone, once a node has a connection to a master. This is currently done in scriptland + // - check FIXME. if ( zeek_table_manager ) - MakeMaster(storename, backend, broker::backend_options{}); - else { - MakeClone(storename); + auto backend = bro_broker::to_backend_type(e); + MakeMaster(storename, backend, broker::backend_options{}); } } } diff --git a/src/zeek.bif b/src/zeek.bif index d63bf1453f..6a7fc740d8 100644 --- a/src/zeek.bif +++ b/src/zeek.bif @@ -1955,9 +1955,10 @@ function global_ids%(%): id_table rec->Assign(3, val_mgr->Bool(id->IsEnumConst())); rec->Assign(4, val_mgr->Bool(id->IsOption())); rec->Assign(5, val_mgr->Bool(id->IsRedefinable())); + rec->Assign(6, val_mgr->Bool(id->GetAttr(zeek::detail::ATTR_BACKEND) != zeek::detail::Attr::nil)); if ( id->HasVal() ) - rec->Assign(6, id->GetVal()); + rec->Assign(7, id->GetVal()); auto id_name = make_intrusive(id->Name()); ids->Assign(std::move(id_name), std::move(rec)); diff --git a/testing/btest/Baseline/broker.store.brokerstore-backend-simple/clone.out b/testing/btest/Baseline/broker.store.brokerstore-backend-simple/clone.out new file mode 100644 index 0000000000..06d6a343ba --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-backend-simple/clone.out @@ -0,0 +1,18 @@ +{ +[b] = 3, +[whatever] = 5, +[a] = 3 +} +{ +hi +} +{ +[b] = [a=2, b=d, c={ +elem1, +elem2 +}], +[a] = [a=1, b=c, c={ +elem1, +elem2 +}] +} diff --git a/testing/btest/Baseline/broker.store.brokerstore-backend-simple/master.out b/testing/btest/Baseline/broker.store.brokerstore-backend-simple/master.out new file mode 100644 index 0000000000..06d6a343ba --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-backend-simple/master.out @@ -0,0 +1,18 @@ +{ +[b] = 3, +[whatever] = 5, +[a] = 3 +} +{ +hi +} +{ +[b] = [a=2, b=d, c={ +elem1, +elem2 +}], +[a] = [a=1, b=c, c={ +elem1, +elem2 +}] +} diff --git a/testing/btest/broker/store/brokerstore-backend-simple.zeek b/testing/btest/broker/store/brokerstore-backend-simple.zeek index 1382fdc7e3..f21906302d 100644 --- a/testing/btest/broker/store/brokerstore-backend-simple.zeek +++ b/testing/btest/broker/store/brokerstore-backend-simple.zeek @@ -1,13 +1,30 @@ -# @TEST-PORT: BROKER_PORT +# @TEST-PORT: BROKER_PORT1 +# @TEST-PORT: BROKER_PORT2 +# @TEST-PORT: BROKER_PORT3 -# @TEST-EXEC: btest-bg-run master "zeek -B broker -b ../master.zeek >../master.out" -# @TEST-EXEC: btest-bg-run clone "zeek -B broker -b ../clone.zeek >../clone.out" +# @TEST-EXEC: btest-bg-run manager-1 "ZEEKPATH=$ZEEKPATH:.. CLUSTER_NODE=manager-1 zeek -B broker ../master.zeek >../master.out" +# @TEST-EXEC: btest-bg-run worker-1 "ZEEKPATH=$ZEEKPATH:.. CLUSTER_NODE=worker-1 zeek -B broker ../clone.zeek >../clone.out" +# @TEST-EXEC: btest-bg-run worker-2 "ZEEKPATH=$ZEEKPATH:.. CLUSTER_NODE=worker-2 zeek -B broker ../clone.zeek >../clone2.out" # @TEST-EXEC: btest-bg-wait 15 # +# @TEST-EXEC: btest-diff master.out # @TEST-EXEC: btest-diff clone.out +# @TEST-EXEC: diff master.out clone.out +# @TEST-EXEC: diff master.out clone2.out + +@TEST-START-FILE cluster-layout.zeek +redef Cluster::nodes = { + ["manager-1"] = [$node_type=Cluster::MANAGER, $ip=127.0.0.1, $p=to_port(getenv("BROKER_PORT1"))], + ["worker-1"] = [$node_type=Cluster::WORKER, $ip=127.0.0.1, $p=to_port(getenv("BROKER_PORT2")), $manager="manager-1", $interface="eth0"], + ["worker-2"] = [$node_type=Cluster::WORKER, $ip=127.0.0.1, $p=to_port(getenv("BROKER_PORT3")), $manager="manager-1", $interface="eth0"], +}; +@TEST-END-FILE + @TEST-START-FILE master.zeek redef exit_only_after_terminate = T; +redef Log::enable_local_logging = T; +redef Log::default_rotation_interval = 0secs; module TestModule; @@ -23,12 +40,6 @@ global r: table[string] of testrec &backend=Broker::MEMORY; event zeek_init() { - Broker::listen("127.0.0.1", to_port(getenv("BROKER_PORT"))); - } - -event insert_stuff() - { - print "Inserting stuff"; t["a"] = 5; delete t["a"]; add s["hi"]; @@ -46,12 +57,6 @@ event insert_stuff() print r; } -event Broker::peer_added(endpoint: Broker::EndpointInfo, msg: string) - { - print "Peer added ", endpoint; - schedule 3secs { insert_stuff() }; - } - event Broker::peer_lost(endpoint: Broker::EndpointInfo, msg: string) { terminate(); @@ -61,7 +66,8 @@ event Broker::peer_lost(endpoint: Broker::EndpointInfo, msg: string) @TEST-START-FILE clone.zeek redef exit_only_after_terminate = T; -redef Broker::auto_store_master = F; +redef Log::enable_local_logging = T; +redef Log::default_rotation_interval = 0secs; module TestModule; @@ -76,11 +82,6 @@ global s: set[string] &backend=Broker::MEMORY; global r: table[string] of testrec &backend=Broker::MEMORY; -event zeek_init() - { - Broker::peer("127.0.0.1", to_port(getenv("BROKER_PORT"))); - } - event dump_tables() { print t; @@ -89,9 +90,9 @@ event dump_tables() terminate(); } -event Broker::peer_added(endpoint: Broker::EndpointInfo, msg: string) +event Cluster::node_up(name: string, id: string) { - print "Peer added"; - schedule 5secs { dump_tables() }; + #print "node up", name; + schedule 4secs { dump_tables() }; } @TEST-END-FILE From f6251e62a0814d9d95d036d18bb9bee3d8931cd0 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Wed, 1 Jul 2020 17:10:43 -0700 Subject: [PATCH 15/26] BrokerStore<->Zeek tables: allow setting storage location & tests With this, the basic functionality of &backend seems to be working. It is not yet integrated with zeekctl, one has to manually specify the storage location for the sqlite files somewhere when using sqlite. Usage for memory stores: global table_to_share: table[string] of count &backend=Broker::MEMORY; Usage for sqlite stores: redef Broker::auto_store_db_directory = "[path]"; global table_to_share: table[string] of count &backend=Broker::SQLITE; In both cases, the cluster should automatically sync to changes done by any node. When using sqlite, data should also be saved to disk and re-loaded on startup. --- src/broker/Manager.cc | 25 +++- .../clone.out | 18 +++ .../master.out | 18 +++ .../clone.out | 18 +++ .../master.out | 18 +++ .../brokerstore-backend-simple-reverse.zeek | 135 ++++++++++++++++++ .../store/brokerstore-backend-sqlite.zeek | 123 ++++++++++++++++ 7 files changed, 349 insertions(+), 6 deletions(-) create mode 100644 testing/btest/Baseline/broker.store.brokerstore-backend-simple-reverse/clone.out create mode 100644 testing/btest/Baseline/broker.store.brokerstore-backend-simple-reverse/master.out create mode 100644 testing/btest/Baseline/broker.store.brokerstore-backend-sqlite/clone.out create mode 100644 testing/btest/Baseline/broker.store.brokerstore-backend-sqlite/master.out create mode 100644 testing/btest/broker/store/brokerstore-backend-simple-reverse.zeek create mode 100644 testing/btest/broker/store/brokerstore-backend-sqlite.zeek diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index b897083a40..2dd32c2713 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -235,16 +235,29 @@ void Manager::InitializeBrokerStoreForwarding() id->GetVal()->AsTableVal()->SetBrokerStore(storename); AddForwardedStore(storename, {NewRef{}, id->GetVal()->AsTableVal()}); - // we only create masters here. For clones, we do all the work of setting up // the forwarding - but we do not try to initialize the clone. We can only initialize // the clone, once a node has a connection to a master. This is currently done in scriptland // - check FIXME. - if ( zeek_table_manager ) - { - auto backend = bro_broker::to_backend_type(e); - MakeMaster(storename, backend, broker::backend_options{}); - } + if ( ! zeek_table_manager ) + continue; + + auto backend = bro_broker::to_backend_type(e); + auto suffix = ".store"; + + switch ( backend ) { + case broker::backend::sqlite: + suffix = ".sqlite"; + break; + case broker::backend::rocksdb: + suffix = ".rocksdb"; + break; + default: + break; + } + auto path = zeek_table_db_directory + "/" + storename + suffix; + + MakeMaster(storename, backend, broker::backend_options{{"path", path}}); } } } diff --git a/testing/btest/Baseline/broker.store.brokerstore-backend-simple-reverse/clone.out b/testing/btest/Baseline/broker.store.brokerstore-backend-simple-reverse/clone.out new file mode 100644 index 0000000000..06d6a343ba --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-backend-simple-reverse/clone.out @@ -0,0 +1,18 @@ +{ +[b] = 3, +[whatever] = 5, +[a] = 3 +} +{ +hi +} +{ +[b] = [a=2, b=d, c={ +elem1, +elem2 +}], +[a] = [a=1, b=c, c={ +elem1, +elem2 +}] +} diff --git a/testing/btest/Baseline/broker.store.brokerstore-backend-simple-reverse/master.out b/testing/btest/Baseline/broker.store.brokerstore-backend-simple-reverse/master.out new file mode 100644 index 0000000000..06d6a343ba --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-backend-simple-reverse/master.out @@ -0,0 +1,18 @@ +{ +[b] = 3, +[whatever] = 5, +[a] = 3 +} +{ +hi +} +{ +[b] = [a=2, b=d, c={ +elem1, +elem2 +}], +[a] = [a=1, b=c, c={ +elem1, +elem2 +}] +} diff --git a/testing/btest/Baseline/broker.store.brokerstore-backend-sqlite/clone.out b/testing/btest/Baseline/broker.store.brokerstore-backend-sqlite/clone.out new file mode 100644 index 0000000000..06d6a343ba --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-backend-sqlite/clone.out @@ -0,0 +1,18 @@ +{ +[b] = 3, +[whatever] = 5, +[a] = 3 +} +{ +hi +} +{ +[b] = [a=2, b=d, c={ +elem1, +elem2 +}], +[a] = [a=1, b=c, c={ +elem1, +elem2 +}] +} diff --git a/testing/btest/Baseline/broker.store.brokerstore-backend-sqlite/master.out b/testing/btest/Baseline/broker.store.brokerstore-backend-sqlite/master.out new file mode 100644 index 0000000000..06d6a343ba --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-backend-sqlite/master.out @@ -0,0 +1,18 @@ +{ +[b] = 3, +[whatever] = 5, +[a] = 3 +} +{ +hi +} +{ +[b] = [a=2, b=d, c={ +elem1, +elem2 +}], +[a] = [a=1, b=c, c={ +elem1, +elem2 +}] +} diff --git a/testing/btest/broker/store/brokerstore-backend-simple-reverse.zeek b/testing/btest/broker/store/brokerstore-backend-simple-reverse.zeek new file mode 100644 index 0000000000..64e8363730 --- /dev/null +++ b/testing/btest/broker/store/brokerstore-backend-simple-reverse.zeek @@ -0,0 +1,135 @@ +# @TEST-PORT: BROKER_PORT1 +# @TEST-PORT: BROKER_PORT2 +# @TEST-PORT: BROKER_PORT3 + +# @TEST-EXEC: btest-bg-run manager-1 "ZEEKPATH=$ZEEKPATH:.. CLUSTER_NODE=manager-1 zeek -B broker ../master.zeek >../master.out" +# @TEST-EXEC: btest-bg-run worker-1 "ZEEKPATH=$ZEEKPATH:.. CLUSTER_NODE=worker-1 zeek -B broker ../clone.zeek >../clone.out" +# @TEST-EXEC: btest-bg-run worker-2 "ZEEKPATH=$ZEEKPATH:.. CLUSTER_NODE=worker-2 zeek -B broker ../clone2.zeek >../clone2.out" +# @TEST-EXEC: btest-bg-wait 15 +# +# @TEST-EXEC: btest-diff master.out +# @TEST-EXEC: btest-diff clone.out +# @TEST-EXEC: diff master.out clone.out +# @TEST-EXEC: diff master.out clone2.out + +@TEST-START-FILE cluster-layout.zeek +redef Cluster::nodes = { + ["manager-1"] = [$node_type=Cluster::MANAGER, $ip=127.0.0.1, $p=to_port(getenv("BROKER_PORT1"))], + ["worker-1"] = [$node_type=Cluster::WORKER, $ip=127.0.0.1, $p=to_port(getenv("BROKER_PORT2")), $manager="manager-1", $interface="eth0"], + ["worker-2"] = [$node_type=Cluster::WORKER, $ip=127.0.0.1, $p=to_port(getenv("BROKER_PORT3")), $manager="manager-1", $interface="eth0"], +}; +@TEST-END-FILE + + +@TEST-START-FILE master.zeek +redef exit_only_after_terminate = T; +redef Log::enable_local_logging = T; +redef Log::default_rotation_interval = 0secs; + +module TestModule; + +type testrec: record { + a: count; + b: string; + c: set[string]; +}; + +global t: table[string] of count &backend=Broker::MEMORY; +global s: set[string] &backend=Broker::MEMORY; +global r: table[string] of testrec &backend=Broker::MEMORY; + +event zeek_init() + { + } + +event Broker::peer_lost(endpoint: Broker::EndpointInfo, msg: string) + { + print t; + print s; + print r; + terminate(); + } + +@TEST-END-FILE + +@TEST-START-FILE clone.zeek +redef exit_only_after_terminate = T; +redef Log::enable_local_logging = T; +redef Log::default_rotation_interval = 0secs; + +module TestModule; + +type testrec: record { + a: count; + b: string; + c: set[string]; +}; + +global t: table[string] of count &backend=Broker::MEMORY; +global s: set[string] &backend=Broker::MEMORY; +global r: table[string] of testrec &backend=Broker::MEMORY; + +event terminate_me() + { + terminate(); + } + +event dump_tables() + { + t["a"] = 5; + delete t["a"]; + add s["hi"]; + t["a"] = 2; + t["a"] = 3; + t["b"] = 3; + t["c"] = 4; + t["whatever"] = 5; + delete t["c"]; + r["a"] = testrec($a=1, $b="b", $c=set("elem1", "elem2")); + r["a"] = testrec($a=1, $b="c", $c=set("elem1", "elem2")); + r["b"] = testrec($a=2, $b="d", $c=set("elem1", "elem2")); + print t; + print s; + print r; + schedule 2sec { terminate_me() }; + } + +event Cluster::node_up(name: string, id: string) + { + #print "node up", name; + schedule 1secs { dump_tables() }; + } +@TEST-END-FILE + +@TEST-START-FILE clone2.zeek +redef exit_only_after_terminate = T; +redef Log::enable_local_logging = T; +redef Log::default_rotation_interval = 0secs; + +module TestModule; + +type testrec: record { + a: count; + b: string; + c: set[string]; +}; + +global t: table[string] of count &backend=Broker::MEMORY; +global s: set[string] &backend=Broker::MEMORY; +global r: table[string] of testrec &backend=Broker::MEMORY; + +event dump_tables() + { + print t; + print s; + print r; + terminate(); + } + +event Cluster::node_up(name: string, id: string) + { + #print "node up", name; + schedule 4secs { dump_tables() }; + } +@TEST-END-FILE + diff --git a/testing/btest/broker/store/brokerstore-backend-sqlite.zeek b/testing/btest/broker/store/brokerstore-backend-sqlite.zeek new file mode 100644 index 0000000000..0eb728fe22 --- /dev/null +++ b/testing/btest/broker/store/brokerstore-backend-sqlite.zeek @@ -0,0 +1,123 @@ +# @TEST-PORT: BROKER_PORT1 +# @TEST-PORT: BROKER_PORT2 +# @TEST-PORT: BROKER_PORT3 + +# @TEST-EXEC: zeek preseed-sqlite.zeek; +# @TEST-EXEC: btest-bg-run manager-1 "ZEEKPATH=$ZEEKPATH:.. CLUSTER_NODE=manager-1 zeek -B broker ../master.zeek >../master.out" +# @TEST-EXEC: btest-bg-run worker-1 "ZEEKPATH=$ZEEKPATH:.. CLUSTER_NODE=worker-1 zeek -B broker ../clone.zeek >../clone.out" +# @TEST-EXEC: btest-bg-run worker-2 "ZEEKPATH=$ZEEKPATH:.. CLUSTER_NODE=worker-2 zeek -B broker ../clone.zeek >../clone2.out" +# @TEST-EXEC: btest-bg-wait 15 +# +# @TEST-EXEC: btest-diff master.out +# @TEST-EXEC: btest-diff clone.out +# @TEST-EXEC: diff master.out clone.out +# @TEST-EXEC: diff master.out clone2.out + +@TEST-START-FILE cluster-layout.zeek +redef Cluster::nodes = { + ["manager-1"] = [$node_type=Cluster::MANAGER, $ip=127.0.0.1, $p=to_port(getenv("BROKER_PORT1"))], + ["worker-1"] = [$node_type=Cluster::WORKER, $ip=127.0.0.1, $p=to_port(getenv("BROKER_PORT2")), $manager="manager-1", $interface="eth0"], + ["worker-2"] = [$node_type=Cluster::WORKER, $ip=127.0.0.1, $p=to_port(getenv("BROKER_PORT3")), $manager="manager-1", $interface="eth0"], +}; +@TEST-END-FILE + +@TEST-START-FILE preseed-sqlite.zeek + +module TestModule; + +type testrec: record { + a: count; + b: string; + c: set[string]; +}; + +global t: table[string] of count &backend=Broker::SQLITE; +global s: set[string] &backend=Broker::SQLITE; +global r: table[string] of testrec &backend=Broker::SQLITE; + +event zeek_init() + { + t["a"] = 5; + delete t["a"]; + add s["hi"]; + t["a"] = 2; + t["a"] = 3; + t["b"] = 3; + t["c"] = 4; + t["whatever"] = 5; + delete t["c"]; + r["a"] = testrec($a=1, $b="b", $c=set("elem1", "elem2")); + r["a"] = testrec($a=1, $b="c", $c=set("elem1", "elem2")); + r["b"] = testrec($a=2, $b="d", $c=set("elem1", "elem2")); + print t; + print s; + print r; + } + +@TEST-END-FILE + +@TEST-START-FILE master.zeek +redef exit_only_after_terminate = T; +redef Log::enable_local_logging = T; +redef Log::default_rotation_interval = 0secs; + +module TestModule; + +type testrec: record { + a: count; + b: string; + c: set[string]; +}; + +global t: table[string] of count &backend=Broker::SQLITE; +global s: set[string] &backend=Broker::SQLITE; +global r: table[string] of testrec &backend=Broker::SQLITE; + +redef Broker::auto_store_db_directory = ".."; + +event zeek_init() + { + print t; + print s; + print r; + } + +event Broker::peer_lost(endpoint: Broker::EndpointInfo, msg: string) + { + terminate(); + } + +@TEST-END-FILE + +@TEST-START-FILE clone.zeek +redef exit_only_after_terminate = T; +redef Log::enable_local_logging = T; +redef Log::default_rotation_interval = 0secs; + +module TestModule; + +type testrec: record { + a: count; + b: string; + c: set[string]; +}; + +global t: table[string] of count &backend=Broker::MEMORY; +global s: set[string] &backend=Broker::MEMORY; +global r: table[string] of testrec &backend=Broker::MEMORY; + + +event dump_tables() + { + print t; + print s; + print r; + terminate(); + } + +event Cluster::node_up(name: string, id: string) + { + #print "node up", name; + schedule 4secs { dump_tables() }; + } +@TEST-END-FILE From 3eac12b40d13ac5ee1fa858eddf24cc953f97603 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Thu, 9 Jul 2020 19:43:45 +0000 Subject: [PATCH 16/26] BrokerStore<->Zeek Tables Fix a few small test failures. --- scripts/base/frameworks/cluster/broker-stores.zeek | 1 + testing/btest/Baseline/coverage.init-default/missing_loads | 1 + 2 files changed, 2 insertions(+) diff --git a/scripts/base/frameworks/cluster/broker-stores.zeek b/scripts/base/frameworks/cluster/broker-stores.zeek index 11155e5387..112b53db58 100644 --- a/scripts/base/frameworks/cluster/broker-stores.zeek +++ b/scripts/base/frameworks/cluster/broker-stores.zeek @@ -1,3 +1,4 @@ +@load ./main module Broker; diff --git a/testing/btest/Baseline/coverage.init-default/missing_loads b/testing/btest/Baseline/coverage.init-default/missing_loads index 893a603972..8d6a158c72 100644 --- a/testing/btest/Baseline/coverage.init-default/missing_loads +++ b/testing/btest/Baseline/coverage.init-default/missing_loads @@ -1,3 +1,4 @@ +-./frameworks/cluster/broker-stores.zeek -./frameworks/cluster/nodes/logger.zeek -./frameworks/cluster/nodes/manager.zeek -./frameworks/cluster/nodes/proxy.zeek From 41dd7df69a14b565676779d5ed71c89cfa3b9ac3 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Thu, 9 Jul 2020 14:31:59 -0700 Subject: [PATCH 17/26] BrokerStore<->Zeek table: adopt to recent Zeek API changes --- src/Attr.cc | 4 ++-- src/Val.cc | 13 ++++++------- src/Val.h | 2 +- src/broker/Manager.cc | 16 ++++++++-------- src/broker/Manager.h | 4 ++-- src/broker/Store.h | 2 +- 6 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/Attr.cc b/src/Attr.cc index dd85a08b41..04905ef8d4 100644 --- a/src/Attr.cc +++ b/src/Attr.cc @@ -620,7 +620,7 @@ void Attributes::CheckAttr(Attr* a) } // Temporary since Broker does not support ListVals - and we cannot easily convert to set/vector - if ( type->AsTableType()->IndexTypes().size() != 1 ) + if ( type->AsTableType()->GetIndexTypes().size() != 1 ) { Error("&backend only supports one-element set/table indexes"); } @@ -655,7 +655,7 @@ void Attributes::CheckAttr(Attr* a) } // Temporary since Broker does not support ListVals - and we cannot easily convert to set/vector - if ( type->AsTableType()->IndexTypes().size() != 1 ) + if ( type->AsTableType()->GetIndexTypes().size() != 1 ) { Error("&broker_store only supports one-element set/table indexes"); } diff --git a/src/Val.cc b/src/Val.cc index f130ce720a..1945cdf5d7 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -2151,7 +2151,6 @@ void TableVal::CallChangeFunc(const ValPtr& index, in_change_func = false; } -<<<<<<< HEAD void TableVal::SendToStore(const Val* index, const TableEntryVal* new_entry_val, OnChangeType tpe) { if ( broker_store.empty() || ! index ) @@ -2171,7 +2170,7 @@ void TableVal::SendToStore(const Val* index, const TableEntryVal* new_entry_val, { if ( index->AsListVal()->Length() != 1 ) { - builtin_error("table with complex index not supported for &broker_store"); + zeek::emit_builtin_error("table with complex index not supported for &broker_store"); return; } @@ -2188,7 +2187,7 @@ void TableVal::SendToStore(const Val* index, const TableEntryVal* new_entry_val, if ( ! broker_index ) { - builtin_error("invalid Broker data conversation for table index"); + zeek::emit_builtin_error("invalid Broker data conversation for table index"); return; } @@ -2224,14 +2223,14 @@ void TableVal::SendToStore(const Val* index, const TableEntryVal* new_entry_val, { if ( ! new_entry_val ) { - builtin_error("did not receive new value for broker-store send operation"); + zeek::emit_builtin_error("did not receive new value for broker-store send operation"); return; } auto new_value = new_entry_val->GetVal().get(); auto broker_val = bro_broker::val_to_data(new_value); if ( ! broker_val ) { - builtin_error("invalid Broker data conversation for table value"); + zeek::emit_builtin_error("invalid Broker data conversation for table value"); return; } handle->store.put(std::move(*broker_index), std::move(*broker_val), expiry); @@ -2249,11 +2248,11 @@ void TableVal::SendToStore(const Val* index, const TableEntryVal* new_entry_val, } catch ( InterpreterException& e ) { - builtin_error("The previous error was encountered while trying to resolve the &broker_store attribute of the set/table. Potentially the Broker::Store has not been initialized before being used."); + zeek::emit_builtin_error("The previous error was encountered while trying to resolve the &broker_store attribute of the set/table. Potentially the Broker::Store has not been initialized before being used."); } } -ValPtr TableVal::Remove(const Val& index, bool broker_forward)) +ValPtr TableVal::Remove(const Val& index, bool broker_forward) { auto k = MakeHashKey(index); // this is totally cheating around the fact that we need a Intrusive pointer. diff --git a/src/Val.h b/src/Val.h index ffa4d0eeb8..dd14abc9a1 100644 --- a/src/Val.h +++ b/src/Val.h @@ -1058,7 +1058,7 @@ protected: enum OnChangeType { ELEMENT_NEW, ELEMENT_CHANGED, ELEMENT_REMOVED, ELEMENT_EXPIRED }; // Calls &change_func. - void CallChangeFunc(const Val* index, const ValPtr& old_value, + void CallChangeFunc(const ValPtr& index, const ValPtr& old_value, OnChangeType tpe); // Sends data on to backing Broker Store diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index 6ae5a9c00b..e9918d11a1 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -222,7 +222,7 @@ void Manager::InitPostScript() void Manager::InitializeBrokerStoreForwarding() { - const auto& globals = global_scope()->Vars(); + const auto& globals = zeek::detail::global_scope()->Vars(); for ( const auto& global : globals ) { @@ -233,7 +233,7 @@ void Manager::InitializeBrokerStoreForwarding() auto e = static_cast(attr->GetExpr()->Eval(nullptr)->AsEnum()); auto storename = std::string("___sync_store_") + global.first; id->GetVal()->AsTableVal()->SetBrokerStore(storename); - AddForwardedStore(storename, {NewRef{}, id->GetVal()->AsTableVal()}); + AddForwardedStore(storename, {zeek::NewRef{}, id->GetVal()->AsTableVal()}); // we only create masters here. For clones, we do all the work of setting up // the forwarding - but we do not try to initialize the clone. We can only initialize @@ -1024,7 +1024,7 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) } // FIXME: expiry! - const auto& its = table->GetType()->AsTableType()->IndexTypes(); + const auto& its = table->GetType()->AsTableType()->GetIndexTypes(); assert( its.size() == 1 ); auto zeek_key = data_to_val(std::move(key), its[0].get()); if ( ! zeek_key ) @@ -1073,7 +1073,7 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) return; } - const auto& its = table->GetType()->AsTableType()->IndexTypes(); + const auto& its = table->GetType()->AsTableType()->GetIndexTypes(); assert( its.size() == 1 ); auto zeek_key = data_to_val(std::move(key), its[0].get()); if ( ! zeek_key ) @@ -1113,7 +1113,7 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) auto key = erase.key(); DBG_LOG(DBG_BROKER, "Store %s: Erase key %s", erase.store_id().c_str(), to_string(key).c_str()); - const auto& its = table->GetType()->AsTableType()->IndexTypes(); + const auto& its = table->GetType()->AsTableType()->GetIndexTypes(); assert( its.size() == 1 ); auto zeek_key = data_to_val(std::move(key), its[0].get()); if ( ! zeek_key ) @@ -1677,7 +1677,7 @@ void Manager::BrokerStoreToZeekTable(const std::string& name, const StoreHandleV auto set = caf::get_if(&(keys->get_data())); auto table = handle->forward_to; - const auto& its = table->GetType()->AsTableType()->IndexTypes(); + const auto& its = table->GetType()->AsTableType()->GetIndexTypes(); bool is_set = table->GetType()->IsSet(); assert( its.size() == 1 ); for ( const auto key : *set ) @@ -1804,7 +1804,7 @@ const Stats& Manager::GetStatistics() return statistics; } -bool Manager::AddForwardedStore(const std::string& name, IntrusivePtr table) +bool Manager::AddForwardedStore(const std::string& name, zeek::IntrusivePtr table) { if ( forwarded_stores.find(name) != forwarded_stores.end() ) { @@ -1812,7 +1812,7 @@ bool Manager::AddForwardedStore(const std::string& name, IntrusivePtr return false; } - DBG_LOG(DBG_BROKER, "Adding table forward for data store %s", name.c_str()); + DBG_LOG(DBG_BROKER, "Adding table forward for data store %s", name.c_str()); forwarded_stores.emplace(name, table); CheckForwarding(name); diff --git a/src/broker/Manager.h b/src/broker/Manager.h index a10e8220cb..513412affe 100644 --- a/src/broker/Manager.h +++ b/src/broker/Manager.h @@ -303,7 +303,7 @@ public: */ StoreHandleVal* LookupStore(const std::string& name); - bool AddForwardedStore(const std::string& name, IntrusivePtr table); + bool AddForwardedStore(const std::string& name, zeek::IntrusivePtr table); /** * Close and unregister a data store. Any existing references to the @@ -399,7 +399,7 @@ private: std::string default_log_topic_prefix; std::shared_ptr bstate; std::unordered_map data_stores; - std::unordered_map> forwarded_stores; + std::unordered_map> forwarded_stores; std::unordered_map pending_queries; std::vector forwarded_prefixes; diff --git a/src/broker/Store.h b/src/broker/Store.h index 3a8b9d853e..4e982b2c86 100644 --- a/src/broker/Store.h +++ b/src/broker/Store.h @@ -123,7 +123,7 @@ public: broker::store::proxy proxy; broker::publisher_id store_pid; // Zeek table that events are forwarded to. - IntrusivePtr forward_to; + zeek::IntrusivePtr forward_to; protected: From 2b2a40f49c7296c763ac814f9cc419c6f3da5e69 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Fri, 10 Jul 2020 16:58:34 -0700 Subject: [PATCH 18/26] Zeek Table<->Brokerstore: cleanup, documentation, small fixes This commit adds script/c++ documentation and fixes a few loose ends. It also adds tests for corner cases and massively improves error messages. This also actually introduces type-compatibility checking and introduces a new attribute that lets a user override this if they really know what they are doing. I am not quite sure if we should really let that stay in - but it can be very convenient to have this functionality. One test is continuing to fail - the expiry test is very flaky. This is, I think, caused by delays of the broker store forwarding. I am unsure if we can actually do anything about that. --- NEWS | 14 ++++ scripts/base/frameworks/broker/store.zeek | 6 ++ .../frameworks/cluster/broker-stores.zeek | 14 ++++ scripts/base/init-bare.zeek | 2 +- src/Attr.cc | 47 +++++++++++-- src/Attr.h | 1 + src/Val.cc | 4 +- src/Val.h | 10 ++- src/broker/Manager.cc | 33 +++++---- src/broker/Manager.h | 13 +++- src/parse.y | 9 ++- src/scan.l | 1 + .../.stderr | 5 ++ .../worker-1..stderr | 3 + .../.stderr | 1 + .../broker/store/brokerstore-attr-clone.zeek | 6 +- .../broker/store/brokerstore-attr-expire.zeek | 4 +- .../brokerstore-attr-persistence-clone.zeek | 6 +- .../store/brokerstore-attr-persistence.zeek | 4 +- .../broker/store/brokerstore-attr-simple.zeek | 4 +- .../store/brokerstore-backend-invalid.zeek | 16 +++++ ...okerstore-backend-simple-incompatible.zeek | 67 +++++++++++++++++++ .../brokerstore-backend-simple-reverse.zeek | 6 +- .../store/brokerstore-backend-simple.zeek | 4 +- ...okerstore-backend-sqlite-incompatible.zeek | 38 +++++++++++ .../store/brokerstore-backend-sqlite.zeek | 6 +- 26 files changed, 271 insertions(+), 53 deletions(-) create mode 100644 testing/btest/Baseline/broker.store.brokerstore-backend-invalid/.stderr create mode 100644 testing/btest/Baseline/broker.store.brokerstore-backend-simple-incompatible/worker-1..stderr create mode 100644 testing/btest/Baseline/broker.store.brokerstore-backend-sqlite-incompatible/.stderr create mode 100644 testing/btest/broker/store/brokerstore-backend-invalid.zeek create mode 100644 testing/btest/broker/store/brokerstore-backend-simple-incompatible.zeek create mode 100644 testing/btest/broker/store/brokerstore-backend-sqlite-incompatible.zeek diff --git a/NEWS b/NEWS index 1a902c05a6..00849e701f 100644 --- a/NEWS +++ b/NEWS @@ -55,6 +55,20 @@ New Functionality for which no connection state information is available in the core anymore. These cases will raise the new ``expired_conn_weird`` event. +- Broker Store table synchronization (Experimental). + + Zeek now supports synchronizing tables/sets across clusters using a backing broker + store. The same feature also allows persistent storage of data in tables/sets + over Zeek restarts. This feature is implemented using the new ```&backend``` attribute. + + To synchronize a table over a cluster, you can e.g. use: + + ```global t: table[string] of count &backend=Broker::MEMORY;``` + + This feature is documented in detail here: FIXME. + + Note: this feature is experimental and the syntax/featureset can change in the future. + Changed Functionality --------------------- diff --git a/scripts/base/frameworks/broker/store.zeek b/scripts/base/frameworks/broker/store.zeek index 2456ccbf2f..8a9019bb40 100644 --- a/scripts/base/frameworks/broker/store.zeek +++ b/scripts/base/frameworks/broker/store.zeek @@ -25,8 +25,14 @@ export { ## A negative/zero value indicates to never buffer commands. const default_clone_mutation_buffer_interval = 2min &redef; + ## If set to true, the current node is the master node for broker stores + ## backing zeek tables. By default this value will be automatically set to + ## true in standalone mode, or on the manager node of a cluster. This value + ## should not typically be changed manually. const auto_store_master = T &redef; + ## The directory used for storing persistent database files when using brokerstore + ## backed zeek tables. const auto_store_db_directory = "." &redef; ## Whether a data store query could be completed or not. diff --git a/scripts/base/frameworks/cluster/broker-stores.zeek b/scripts/base/frameworks/cluster/broker-stores.zeek index 112b53db58..1513e6776d 100644 --- a/scripts/base/frameworks/cluster/broker-stores.zeek +++ b/scripts/base/frameworks/cluster/broker-stores.zeek @@ -1,11 +1,23 @@ +##! This script deals with the cluster parts of broker backed zeek tables. +##! It makes sure that the master store is set correctly and that clones +##! are automatically created on the non-manager nodes. + +# Note - this script should become unnecessary in the future, when we just can +# speculatively attach clones. This should be possible once the new ALM broker +# transport becomes available. + @load ./main module Broker; export { + ## Event that is used by the manager to announce the master stores for zeek backed + ## tables that is uses. global announce_masters: event(masters: set[string]); } +# If we are not the manager - disable automatically generating masters. We will attach +# clones instead. @if ( Cluster::is_enabled() && Cluster::local_node_type() != Cluster::MANAGER ) redef Broker::auto_store_master = F; @endif @@ -24,6 +36,7 @@ event zeek_init() } } +# Send the auto masters we created to the newly connected node event Broker::peer_added(endpoint: Broker::EndpointInfo, msg: string) &priority=1 { if ( ! Cluster::is_enabled() ) @@ -39,6 +52,7 @@ event Broker::announce_masters(masters: set[string]) { for ( i in masters ) { + # this magic name for the store is created in broker/Manager.cc for the manager. local name = "___sync_store_" + i; Broker::create_clone(name); } diff --git a/scripts/base/init-bare.zeek b/scripts/base/init-bare.zeek index 6036a0bc66..aa8c28bd1e 100644 --- a/scripts/base/init-bare.zeek +++ b/scripts/base/init-bare.zeek @@ -734,7 +734,7 @@ type script_id: record { enum_constant: bool; ##< True if the identifier is an enum value. option_value: bool; ##< True if the identifier is an option. redefinable: bool; ##< True if the identifier is declared with the :zeek:attr:`&redef` attribute. - broker_backend: bool; ##< True if the identifier has a broker backend defined using the :zeek:attr:`&backend` attribute. + broker_backend: bool; ##< True if the identifier has a broker backend defined using the :zeek:attr:`&backend` attribute. value: any &optional; ##< The current value of the identifier. }; diff --git a/src/Attr.cc b/src/Attr.cc index 04905ef8d4..3aff6718dc 100644 --- a/src/Attr.cc +++ b/src/Attr.cc @@ -8,6 +8,7 @@ #include "Val.h" #include "IntrusivePtr.h" #include "threading/SerialTypes.h" +#include "input/Manager.h" namespace zeek::detail { @@ -19,8 +20,8 @@ const char* attr_name(AttrTag t) "&read_expire", "&write_expire", "&create_expire", "&raw_output", "&priority", "&group", "&log", "&error_handler", "&type_column", - "(&tracked)", "&on_change", "&broker_store", "&backend", - "&deprecated", + "(&tracked)", "&on_change", "&broker_store", + "&broker_allow_complex_type", "&backend", "&deprecated", }; return attr_names[int(t)]; @@ -450,6 +451,10 @@ void Attributes::CheckAttr(Attr* a) { Error("&broker_store and &read_expire cannot be used simultaneously"); } + if ( Find(ATTR_BACKEND) ) + { + Error("&broker_store and &backend cannot be used simultaneously"); + } } // fallthrough case ATTR_EXPIRE_WRITE: @@ -535,6 +540,10 @@ void Attributes::CheckAttr(Attr* a) { Error("&broker_store and &expire_func cannot be used simultaneously"); } + if ( Find(ATTR_BACKEND ) ) + { + Error("&backend and &expire_func cannot be used simultaneously"); + } } break; @@ -607,7 +616,7 @@ void Attributes::CheckAttr(Attr* a) { if ( ! global_var || type->Tag() != TYPE_TABLE ) { - Error("&broker_store only applicable to global sets/tables"); + Error("&backend only applicable to global sets/tables"); break; } @@ -615,7 +624,7 @@ void Attributes::CheckAttr(Attr* a) // are here. We will do that later - before actually attaching to a broker store if ( a->GetExpr()->GetType()->Tag() != TYPE_ENUM ) { - Error("&broker_store must take an enum argument"); + Error("&backend must take an enum argument"); break; } @@ -625,6 +634,14 @@ void Attributes::CheckAttr(Attr* a) Error("&backend only supports one-element set/table indexes"); } + // Only support atomic types for the moment. + if ( ! type->AsTableType()->IsSet() && + ! input::Manager::IsCompatibleType(type->AsTableType()->Yield().get(), true) && + ! Find(ATTR_BROKER_STORE_ALLOW_COMPLEX) ) + { + Error("&backend only supports atomic types as table value"); + } + if ( Find(ATTR_EXPIRE_FUNC ) ) { Error("&backend and &expire_func cannot be used simultaneously"); @@ -655,11 +672,18 @@ void Attributes::CheckAttr(Attr* a) } // Temporary since Broker does not support ListVals - and we cannot easily convert to set/vector - if ( type->AsTableType()->GetIndexTypes().size() != 1 ) + if ( type->AsTableType()->GetIndexTypes().size() != 1 && ! Find(ATTR_BROKER_STORE_ALLOW_COMPLEX) ) { Error("&broker_store only supports one-element set/table indexes"); } + if ( ! type->AsTableType()->IsSet() && + ! input::Manager::IsCompatibleType(type->AsTableType()->Yield().get(), true) && + ! Find(ATTR_BROKER_STORE_ALLOW_COMPLEX) ) + { + Error("&broker_store only supports atomic types as table value"); + } + if ( Find(ATTR_EXPIRE_FUNC ) ) { Error("&broker_store and &expire_func cannot be used simultaneously"); @@ -668,8 +692,21 @@ void Attributes::CheckAttr(Attr* a) { Error("&broker_store and &read_expire cannot be used simultaneously"); } + if ( Find(ATTR_BACKEND) ) + { + Error("&backend and &broker_store cannot be used simultaneously"); + } break; } + + case ATTR_BROKER_STORE_ALLOW_COMPLEX: + { + if ( type->Tag() != TYPE_TABLE ) + { + Error("&broker_allow_complex_type only applicable to sets/tables"); + break; + } + } case ATTR_TRACKED: // FIXME: Check here for global ID? break; diff --git a/src/Attr.h b/src/Attr.h index 9d52fa8bd2..0628530ad0 100644 --- a/src/Attr.h +++ b/src/Attr.h @@ -42,6 +42,7 @@ enum AttrTag { ATTR_TRACKED, // hidden attribute, tracked by NotifierRegistry ATTR_ON_CHANGE, // for table change tracking ATTR_BROKER_STORE, // for broker-store backed tables + ATTR_BROKER_STORE_ALLOW_COMPLEX, // for broker-store backed tables ATTR_BACKEND, // for broker-store backed tabled ATTR_DEPRECATED, NUM_ATTRS // this item should always be last diff --git a/src/Val.cc b/src/Val.cc index 1945cdf5d7..133ecad543 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -1515,7 +1515,7 @@ void TableVal::SetAttrs(detail::AttributesPtr a) change_func = cf->GetExpr(); auto bs = attrs->Find(zeek::detail::ATTR_BROKER_STORE); - if ( bs && broker_store.empty() ) // this does not mesh well with being updated several times + if ( bs && broker_store.empty() ) { IntrusivePtr c = bs->GetExpr()->Eval(nullptr); assert(c); @@ -2181,8 +2181,6 @@ void TableVal::SendToStore(const Val* index, const TableEntryVal* new_entry_val, index_val = index; } - // FIXME: switch back to just storing tables directly in the broker store? - // me which store a change came from - and this still seems to be missing from the store_events. (Or I am blind). auto broker_index = bro_broker::val_to_data(index_val); if ( ! broker_index ) diff --git a/src/Val.h b/src/Val.h index dd14abc9a1..ef02fbc3f3 100644 --- a/src/Val.h +++ b/src/Val.h @@ -785,7 +785,7 @@ public: * @param new_val The value to assign at the index. For a set, this * must be nullptr. * @param broker_forward Controls if the value will be forwarded to attached - * broker stores. + * broker stores. * @return True if the assignment type-checked. */ bool Assign(ValPtr index, ValPtr new_val, bool broker_forward = true); @@ -799,7 +799,7 @@ public: * @param new_val The value to assign at the index. For a set, this * must be nullptr. * @param broker_forward Controls if the value will be forwarded to attached - * broker stores. + * broker stores. * @return True if the assignment type-checked. */ bool Assign(ValPtr index, std::unique_ptr k, @@ -926,7 +926,7 @@ public: * Remove an element from the table and return it. * @param index The index to remove. * @param broker_forward Controls if the remove operation will be forwarded to attached - * broker stores. + * broker stores. * @return The value associated with the index if it exists, else nullptr. * For a sets that don't really contain associated values, a placeholder * value is returned to differentiate it from non-existent index (nullptr), @@ -1023,6 +1023,10 @@ public: // on zeek::RecordTypes. static void DoneParsing(); + /** + * Sets the name of the broker store that is backing this table. + * @param store store that is backing this table. + */ void SetBrokerStore(const std::string& store) { broker_store = store; } protected: diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index e9918d11a1..cc689bbf90 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -238,7 +238,8 @@ void Manager::InitializeBrokerStoreForwarding() // we only create masters here. For clones, we do all the work of setting up // the forwarding - but we do not try to initialize the clone. We can only initialize // the clone, once a node has a connection to a master. This is currently done in scriptland - // - check FIXME. + // in scripts/base/frameworks/cluster/broker-stores.zeek. Once the ALM transport is ready + // we can change over to doing this here. if ( ! zeek_table_manager ) continue; @@ -956,7 +957,7 @@ void Manager::Process() if ( broker::topics::store_events.prefix_of(topic) ) { - ProcessStoreEvent(topic, std::move(msg)); + ProcessStoreEvent(std::move(msg)); continue; } @@ -995,7 +996,7 @@ void Manager::Process() } } -void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) +void Manager::ProcessStoreEvent(broker::data msg) { if ( auto insert = broker::store_event::insert::make(msg) ) { @@ -1016,20 +1017,18 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) DBG_LOG(DBG_BROKER, "Store %s: Insert: %s:%s (%s:%s)", insert.store_id().c_str(), to_string(insert.key()).c_str(), to_string(insert.value()).c_str(), insert.key().get_type_name(), insert.value().get_type_name()); - if ( table->GetType()->IsSet() && data.get_type() != broker::data::type::none ) { reporter->Error("ProcessStoreEvent Insert got %s when expecting set", data.get_type_name()); return; } - // FIXME: expiry! const auto& its = table->GetType()->AsTableType()->GetIndexTypes(); assert( its.size() == 1 ); - auto zeek_key = data_to_val(std::move(key), its[0].get()); + auto zeek_key = data_to_val(key, its[0].get()); if ( ! zeek_key ) { - reporter->Error("ProcessStoreEvent: failed to convert key"); + reporter->Error("ProcessStoreEvent: could not convert key \"%s\" for store \"%s\" while receiving remote insert. This probably means the tables have different types on different nodes.", to_string(key).c_str(), insert.store_id().c_str()); return; } @@ -1040,10 +1039,10 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) } // it is a table - auto zeek_value = data_to_val(std::move(data), table->GetType()->Yield().get()); + auto zeek_value = data_to_val(data, table->GetType()->Yield().get()); if ( ! zeek_value ) { - reporter->Error("ProcessStoreEvent: failed to convert value"); + reporter->Error("ProcessStoreEvent: could not convert value \"%s\" for key \"%s\" in store \"%s\" while receiving remote insert. This probably means the tables have different types on different nodes.", to_string(data).c_str(), to_string(key).c_str(), insert.store_id().c_str()); return; } table->Assign(zeek_key, zeek_value, false); @@ -1075,10 +1074,10 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) const auto& its = table->GetType()->AsTableType()->GetIndexTypes(); assert( its.size() == 1 ); - auto zeek_key = data_to_val(std::move(key), its[0].get()); + auto zeek_key = data_to_val(key, its[0].get()); if ( ! zeek_key ) { - reporter->Error("ProcessStoreEvent: failed to convert key"); + reporter->Error("ProcessStoreEvent: could not convert key \"%s\" for store \"%s\" while receiving remote update. This probably means the tables have different types on different nodes.", to_string(key).c_str(), insert.store_id().c_str()); return; } @@ -1089,10 +1088,10 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) } // it is a table - auto zeek_value = data_to_val(std::move(data), table->GetType()->Yield().get()); + auto zeek_value = data_to_val(data, table->GetType()->Yield().get()); if ( ! zeek_value ) { - reporter->Error("ProcessStoreEvent: failed to convert value"); + reporter->Error("ProcessStoreEvent: could not convert value \"%s\" for key \"%s\" in store \"%s\" while receiving remote update. This probably means the tables have different types on different nodes.", to_string(data).c_str(), to_string(key).c_str(), insert.store_id().c_str()); return; } table->Assign(zeek_key, zeek_value, false); @@ -1115,17 +1114,17 @@ void Manager::ProcessStoreEvent(const broker::topic& topic, broker::data msg) DBG_LOG(DBG_BROKER, "Store %s: Erase key %s", erase.store_id().c_str(), to_string(key).c_str()); const auto& its = table->GetType()->AsTableType()->GetIndexTypes(); assert( its.size() == 1 ); - auto zeek_key = data_to_val(std::move(key), its[0].get()); + auto zeek_key = data_to_val(key, its[0].get()); if ( ! zeek_key ) { - reporter->Error("ProcessStoreEvent: failed to convert key"); + reporter->Error("ProcessStoreEvent: could not convert key \"%s\" for store \"%s\" while receiving remote erase. This probably means the tables have different types on different nodes.", to_string(key).c_str(), insert.store_id().c_str()); return; } table->Remove(*zeek_key, false); } else if ( auto expire = broker::store_event::expire::make(msg) ) { - // We just ignore expirys - expiring information on the Zeek side is handled by Zeek itself. + // We just ignore expiries - expiring information on the Zeek side is handled by Zeek itself. #ifdef DEBUG // let's only debug log for stores that we know. auto storehandle = broker_mgr->LookupStore(expire.store_id()); @@ -1685,7 +1684,7 @@ void Manager::BrokerStoreToZeekTable(const std::string& name, const StoreHandleV auto zeek_key = data_to_val(key, its[0].get()); if ( ! zeek_key ) { - reporter->Error("Failed to convert key %s while importing broker store to table for store %s. Aborting import.", to_string(key).c_str(), name.c_str()); + reporter->Error("Failed to convert key \"%s\" while importing broker store to table for store \"%s\". Aborting import.", to_string(key).c_str(), name.c_str()); // just abort - this probably means the types are incompatible return; } diff --git a/src/broker/Manager.h b/src/broker/Manager.h index 513412affe..18b6921641 100644 --- a/src/broker/Manager.h +++ b/src/broker/Manager.h @@ -303,6 +303,15 @@ public: */ StoreHandleVal* LookupStore(const std::string& name); + /** + * Register a zeek table that is associated with a broker store that is backing it. This + * causes all changes that happen to the brokerstore in the future to be applied to the zeek + * table. + * A single broker store can only be forwarded to a single table. + * @param name name of the broker store + * @param table pointer to the table/set that is being backed + * @return true on success, false if the named store is already being forwarded. + */ bool AddForwardedStore(const std::string& name, zeek::IntrusivePtr table); /** @@ -350,7 +359,8 @@ public: private: void DispatchMessage(const broker::topic& topic, broker::data msg); - void ProcessStoreEvent(const broker::topic& topic, broker::data msg); + // Process events used for broker store backed zeek tables + void ProcessStoreEvent(broker::data msg); void ProcessEvent(const broker::topic& topic, broker::zeek::Event ev); bool ProcessLogCreate(broker::zeek::LogCreate lc); bool ProcessLogWrite(broker::zeek::LogWrite lw); @@ -359,6 +369,7 @@ private: void ProcessError(broker::error err); void ProcessStoreResponse(StoreHandleVal*, broker::store::response response); void FlushPendingQueries(); + // Initializes the masters for broker backed zeek tables when using the &backend attribute void InitializeBrokerStoreForwarding(); // Check if a broker store is associated to a table on the Zeek side. void CheckForwarding(const std::string& name); diff --git a/src/parse.y b/src/parse.y index 8257c84600..5830dd1445 100644 --- a/src/parse.y +++ b/src/parse.y @@ -5,7 +5,7 @@ // Switching parser table type fixes ambiguity problems. %define lr.type ielr -%expect 123 +%expect 129 %token TOK_ADD TOK_ADD_TO TOK_ADDR TOK_ANY %token TOK_ATENDIF TOK_ATELSE TOK_ATIF TOK_ATIFDEF TOK_ATIFNDEF @@ -24,7 +24,8 @@ %token TOK_ATTR_ADD_FUNC TOK_ATTR_DEFAULT TOK_ATTR_OPTIONAL TOK_ATTR_REDEF %token TOK_ATTR_DEL_FUNC TOK_ATTR_EXPIRE_FUNC %token TOK_ATTR_EXPIRE_CREATE TOK_ATTR_EXPIRE_READ TOK_ATTR_EXPIRE_WRITE -%token TOK_ATTR_RAW_OUTPUT TOK_ATTR_ON_CHANGE TOK_ATTR_BROKER_STORE TOK_ATTR_BACKEND +%token TOK_ATTR_RAW_OUTPUT TOK_ATTR_ON_CHANGE TOK_ATTR_BROKER_STORE +%token TOK_ATTR_BROKER_STORE_ALLOW_COMPLEX TOK_ATTR_BACKEND %token TOK_ATTR_PRIORITY TOK_ATTR_LOG TOK_ATTR_ERROR_HANDLER %token TOK_ATTR_TYPE_COLUMN TOK_ATTR_DEPRECATED @@ -1355,7 +1356,7 @@ attr_list: attr: TOK_ATTR_DEFAULT '=' expr - { $$ = new zeek::detail::Attr(zeek::detail::ATTR_DEFAULT, {zeek::AdoptRef{}, $3}); } + { $$ = new zeek::detail::Attr(zeek::detail::ATTR_DEFAULT, {zeek::AdoptRef{}, $3}); } | TOK_ATTR_OPTIONAL { $$ = new zeek::detail::Attr(zeek::detail::ATTR_OPTIONAL); } | TOK_ATTR_REDEF @@ -1368,6 +1369,8 @@ attr: { $$ = new zeek::detail::Attr(zeek::detail::ATTR_ON_CHANGE, {zeek::AdoptRef{}, $3}); } | TOK_ATTR_BROKER_STORE '=' expr { $$ = new zeek::detail::Attr(zeek::detail::ATTR_BROKER_STORE, {zeek::AdoptRef{}, $3}); } + | TOK_ATTR_BROKER_STORE_ALLOW_COMPLEX + { $$ = new zeek::detail::Attr(zeek::detail::ATTR_BROKER_STORE_ALLOW_COMPLEX); } | TOK_ATTR_BACKEND '=' expr { $$ = new zeek::detail::Attr(zeek::detail::ATTR_BACKEND, {zeek::AdoptRef{}, $3}); } | TOK_ATTR_EXPIRE_FUNC '=' expr diff --git a/src/scan.l b/src/scan.l index 41228eef49..ba25c2de62 100644 --- a/src/scan.l +++ b/src/scan.l @@ -285,6 +285,7 @@ when return TOK_WHEN; &write_expire return TOK_ATTR_EXPIRE_WRITE; &on_change return TOK_ATTR_ON_CHANGE; &broker_store return TOK_ATTR_BROKER_STORE; +&broker_allow_complex_type return TOK_ATTR_BROKER_STORE_ALLOW_COMPLEX; &backend return TOK_ATTR_BACKEND; @deprecated.* { diff --git a/testing/btest/Baseline/broker.store.brokerstore-backend-invalid/.stderr b/testing/btest/Baseline/broker.store.brokerstore-backend-invalid/.stderr new file mode 100644 index 0000000000..7894cdb98f --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-backend-invalid/.stderr @@ -0,0 +1,5 @@ +error in /this/is/a/path/brokerstore-backend-invalid.zeek, line 12: &backend only supports one-element set/table indexes (&backend=Broker::MEMORY) +error in /this/is/a/path/broker.store.brokerstore-backend-invalid/brokerstore-backend-invalid.zeek, line 13: &backend only supports atomic types as table value (&backend=Broker::MEMORY) +error in /this/is/a/path/broker.store.brokerstore-backend-invalid/brokerstore-backend-invalid.zeek, line 14: &backend and &read_expire cannot be used simultaneously (&read_expire=5.0 secs, &backend=Broker::MEMORY) +error in /this/is/a/path/broker.store.brokerstore-backend-invalid/brokerstore-backend-invalid.zeek, line 15: &backend and &broker_store cannot be used simultaneously (&broker_store=store, &backend=Broker::MEMORY) +error in /this/is/a/path/broker.store.brokerstore-backend-invalid/brokerstore-backend-invalid.zeek, line 16: &backend only applicable to global sets/tables (&backend=Broker::MEMORY) diff --git a/testing/btest/Baseline/broker.store.brokerstore-backend-simple-incompatible/worker-1..stderr b/testing/btest/Baseline/broker.store.brokerstore-backend-simple-incompatible/worker-1..stderr new file mode 100644 index 0000000000..8a5d27c280 --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-backend-simple-incompatible/worker-1..stderr @@ -0,0 +1,3 @@ +error: ProcessStoreEvent: could not convert value "b" for key "a" in store "___sync_store_TestModule::s" while receiving remote insert. This probably means the tables have different types on different nodes. +error: ProcessStoreEvent: could not convert key "a" for store "___sync_store_TestModule::t" while receiving remote insert. This probably means the tables have different types on different nodes. +received termination signal diff --git a/testing/btest/Baseline/broker.store.brokerstore-backend-sqlite-incompatible/.stderr b/testing/btest/Baseline/broker.store.brokerstore-backend-sqlite-incompatible/.stderr new file mode 100644 index 0000000000..ce13e7d1a4 --- /dev/null +++ b/testing/btest/Baseline/broker.store.brokerstore-backend-sqlite-incompatible/.stderr @@ -0,0 +1 @@ +error: Failed to convert key "a" while importing broker store to table for store "___sync_store_TestModule::t". Aborting import. diff --git a/testing/btest/broker/store/brokerstore-attr-clone.zeek b/testing/btest/broker/store/brokerstore-attr-clone.zeek index e618196f0d..429c8e6d4f 100644 --- a/testing/btest/broker/store/brokerstore-attr-clone.zeek +++ b/testing/btest/broker/store/brokerstore-attr-clone.zeek @@ -27,7 +27,7 @@ type testrec: record { global t: table[string] of count &broker_store="table"; global s: set[string] &broker_store="set"; -global r: table[string] of testrec &broker_store="rec"; +global r: table[string] of testrec &broker_allow_complex_type &broker_store="rec"; event zeek_init() { @@ -73,7 +73,7 @@ type testrec: record { global t: table[string] of count &broker_store="table"; global s: set[string] &broker_store="set"; -global r: table[string] of testrec &broker_store="rec"; +global r: table[string] of testrec &broker_allow_complex_type &broker_store="rec"; event zeek_init() { @@ -133,7 +133,7 @@ type testrec: record { global t: table[string] of count &broker_store="table"; global s: set[string] &broker_store="set"; -global r: table[string] of testrec &broker_store="rec"; +global r: table[string] of testrec &broker_allow_complex_type &broker_store="rec"; event zeek_init() { diff --git a/testing/btest/broker/store/brokerstore-attr-expire.zeek b/testing/btest/broker/store/brokerstore-attr-expire.zeek index 0b12e4df96..be58d3e005 100644 --- a/testing/btest/broker/store/brokerstore-attr-expire.zeek +++ b/testing/btest/broker/store/brokerstore-attr-expire.zeek @@ -62,7 +62,7 @@ function print_keys() global t: table[string] of count &broker_store="table" &create_expire=4sec &on_change=change_t; global s: table[string] of count &broker_store="set" &write_expire=3sec &on_change=change_s; -global r: table[string] of testrec &broker_store="rec" &write_expire=5sec &on_change=change_r; +global r: table[string] of testrec &broker_allow_complex_type &broker_store="rec" &write_expire=5sec &on_change=change_r; event zeek_init() { @@ -154,7 +154,7 @@ function change_r(tbl: any, tpe: TableChange, idx: string, idxb: testrec) global t: table[string] of count &broker_store="table" &create_expire=4sec &on_change=change_t; global s: table[string] of count &broker_store="set" &write_expire=3sec &on_change=change_s; -global r: table[string] of testrec &broker_store="rec" &write_expire=5sec &on_change=change_r; +global r: table[string] of testrec &broker_allow_complex_type &broker_store="rec" &write_expire=5sec &on_change=change_r; event zeek_init() { diff --git a/testing/btest/broker/store/brokerstore-attr-persistence-clone.zeek b/testing/btest/broker/store/brokerstore-attr-persistence-clone.zeek index 5f6b9ffe87..bece4f119b 100644 --- a/testing/btest/broker/store/brokerstore-attr-persistence-clone.zeek +++ b/testing/btest/broker/store/brokerstore-attr-persistence-clone.zeek @@ -29,7 +29,7 @@ type testrec: record { global t: table[string] of count &broker_store="table"; global s: set[string] &broker_store="set"; -global r: table[string] of testrec &broker_store="rec"; +global r: table[string] of testrec &broker_allow_complex_type &broker_store="rec"; event zeek_init() { @@ -73,7 +73,7 @@ type testrec: record { global t: table[string] of count &broker_store="table"; global s: set[string] &broker_store="set"; -global r: table[string] of testrec &broker_store="rec"; +global r: table[string] of testrec &broker_allow_complex_type &broker_store="rec"; event zeek_init() { @@ -114,7 +114,7 @@ type testrec: record { global t: table[string] of count &broker_store="table"; global s: set[string] &broker_store="set"; -global r: table[string] of testrec &broker_store="rec"; +global r: table[string] of testrec &broker_allow_complex_type &broker_store="rec"; event zeek_init() { diff --git a/testing/btest/broker/store/brokerstore-attr-persistence.zeek b/testing/btest/broker/store/brokerstore-attr-persistence.zeek index 5004b12b91..abd40df407 100644 --- a/testing/btest/broker/store/brokerstore-attr-persistence.zeek +++ b/testing/btest/broker/store/brokerstore-attr-persistence.zeek @@ -24,7 +24,7 @@ type testrec: record { global t: table[string] of count &broker_store="table"; global s: set[string] &broker_store="set"; -global r: table[string] of testrec &broker_store="rec"; +global r: table[string] of testrec &broker_allow_complex_type &broker_store="rec"; event zeek_init() { @@ -66,7 +66,7 @@ type testrec: record { global t: table[string] of count &broker_store="table"; global s: set[string] &broker_store="set"; -global r: table[string] of testrec &broker_store="rec"; +global r: table[string] of testrec &broker_allow_complex_type &broker_store="rec"; event zeek_init() { diff --git a/testing/btest/broker/store/brokerstore-attr-simple.zeek b/testing/btest/broker/store/brokerstore-attr-simple.zeek index ac2bfd2233..95ef10454f 100644 --- a/testing/btest/broker/store/brokerstore-attr-simple.zeek +++ b/testing/btest/broker/store/brokerstore-attr-simple.zeek @@ -23,7 +23,7 @@ type testrec: record { global t: table[string] of count &broker_store="table"; global s: set[string] &broker_store="set"; -global r: table[string] of testrec &broker_store="rec"; +global r: table[string] of testrec &broker_allow_complex_type &broker_store="rec"; event zeek_init() { @@ -83,7 +83,7 @@ type testrec: record { global t: table[string] of count &broker_store="table"; global s: set[string] &broker_store="set"; -global r: table[string] of testrec &broker_store="rec"; +global r: table[string] of testrec &broker_allow_complex_type &broker_store="rec"; event zeek_init() { diff --git a/testing/btest/broker/store/brokerstore-backend-invalid.zeek b/testing/btest/broker/store/brokerstore-backend-invalid.zeek new file mode 100644 index 0000000000..cad1828213 --- /dev/null +++ b/testing/btest/broker/store/brokerstore-backend-invalid.zeek @@ -0,0 +1,16 @@ +# @TEST-EXEC-FAIL: zeek -B broker %INPUT +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr + +module TestModule; + +type testrec: record { + a: count; + b: string; + c: set[string]; +}; + +global a: table[string, count] of count &backend=Broker::MEMORY; +global b: table[string] of testrec &backend=Broker::MEMORY; +global c: table[string] of count &read_expire=5sec &backend=Broker::MEMORY; +global d: table[string] of count &broker_store="store" &backend=Broker::MEMORY; +global f: count &backend=Broker::MEMORY; diff --git a/testing/btest/broker/store/brokerstore-backend-simple-incompatible.zeek b/testing/btest/broker/store/brokerstore-backend-simple-incompatible.zeek new file mode 100644 index 0000000000..ce07488da0 --- /dev/null +++ b/testing/btest/broker/store/brokerstore-backend-simple-incompatible.zeek @@ -0,0 +1,67 @@ +# @TEST-PORT: BROKER_PORT1 +# @TEST-PORT: BROKER_PORT2 +# @TEST-PORT: BROKER_PORT3 + +# @TEST-EXEC: btest-bg-run manager-1 "ZEEKPATH=$ZEEKPATH:.. CLUSTER_NODE=manager-1 zeek -B broker ../master.zeek >../master.out" +# @TEST-EXEC: btest-bg-run worker-1 "ZEEKPATH=$ZEEKPATH:.. CLUSTER_NODE=worker-1 zeek -B broker ../clone.zeek >../clone.out" +# @TEST-EXEC: btest-bg-run worker-2 "ZEEKPATH=$ZEEKPATH:.. CLUSTER_NODE=worker-2 zeek -B broker ../clone.zeek >../clone2.out" +# @TEST-EXEC: btest-bg-wait 15 +# +# @TEST-EXEC: btest-diff worker-1/.stderr + +@TEST-START-FILE cluster-layout.zeek +redef Cluster::nodes = { + ["manager-1"] = [$node_type=Cluster::MANAGER, $ip=127.0.0.1, $p=to_port(getenv("BROKER_PORT1"))], + ["worker-1"] = [$node_type=Cluster::WORKER, $ip=127.0.0.1, $p=to_port(getenv("BROKER_PORT2")), $manager="manager-1", $interface="eth0"], + ["worker-2"] = [$node_type=Cluster::WORKER, $ip=127.0.0.1, $p=to_port(getenv("BROKER_PORT3")), $manager="manager-1", $interface="eth0"], +}; +@TEST-END-FILE + + +@TEST-START-FILE master.zeek +redef exit_only_after_terminate = T; +redef Log::enable_local_logging = T; +redef Log::default_rotation_interval = 0secs; + +module TestModule; + +global t: table[string] of count &backend=Broker::MEMORY; +global s: table[string] of string &backend=Broker::MEMORY; + +event zeek_init() + { + t["a"] = 5; + s["a"] = "b"; + print t; + } + +event Broker::peer_lost(endpoint: Broker::EndpointInfo, msg: string) + { + terminate(); + } + +@TEST-END-FILE + +@TEST-START-FILE clone.zeek +redef exit_only_after_terminate = T; +redef Log::enable_local_logging = T; +redef Log::default_rotation_interval = 0secs; + +module TestModule; + +global t: table[count] of count &backend=Broker::MEMORY; +global s: table[string] of count &backend=Broker::MEMORY; + +event dump_tables() + { + print t; + print s; + terminate(); + } + +event Cluster::node_up(name: string, id: string) + { + #print "node up", name; + schedule 4secs { dump_tables() }; + } +@TEST-END-FILE diff --git a/testing/btest/broker/store/brokerstore-backend-simple-reverse.zeek b/testing/btest/broker/store/brokerstore-backend-simple-reverse.zeek index 64e8363730..8ec17e5f32 100644 --- a/testing/btest/broker/store/brokerstore-backend-simple-reverse.zeek +++ b/testing/btest/broker/store/brokerstore-backend-simple-reverse.zeek @@ -36,7 +36,7 @@ type testrec: record { global t: table[string] of count &backend=Broker::MEMORY; global s: set[string] &backend=Broker::MEMORY; -global r: table[string] of testrec &backend=Broker::MEMORY; +global r: table[string] of testrec &broker_allow_complex_type &backend=Broker::MEMORY; event zeek_init() { @@ -67,7 +67,7 @@ type testrec: record { global t: table[string] of count &backend=Broker::MEMORY; global s: set[string] &backend=Broker::MEMORY; -global r: table[string] of testrec &backend=Broker::MEMORY; +global r: table[string] of testrec &broker_allow_complex_type &backend=Broker::MEMORY; event terminate_me() { @@ -116,7 +116,7 @@ type testrec: record { global t: table[string] of count &backend=Broker::MEMORY; global s: set[string] &backend=Broker::MEMORY; -global r: table[string] of testrec &backend=Broker::MEMORY; +global r: table[string] of testrec &broker_allow_complex_type &backend=Broker::MEMORY; event dump_tables() { diff --git a/testing/btest/broker/store/brokerstore-backend-simple.zeek b/testing/btest/broker/store/brokerstore-backend-simple.zeek index f21906302d..6f47e5d3e0 100644 --- a/testing/btest/broker/store/brokerstore-backend-simple.zeek +++ b/testing/btest/broker/store/brokerstore-backend-simple.zeek @@ -36,7 +36,7 @@ type testrec: record { global t: table[string] of count &backend=Broker::MEMORY; global s: set[string] &backend=Broker::MEMORY; -global r: table[string] of testrec &backend=Broker::MEMORY; +global r: table[string] of testrec &broker_allow_complex_type &backend=Broker::MEMORY; event zeek_init() { @@ -79,7 +79,7 @@ type testrec: record { global t: table[string] of count &backend=Broker::MEMORY; global s: set[string] &backend=Broker::MEMORY; -global r: table[string] of testrec &backend=Broker::MEMORY; +global r: table[string] of testrec &broker_allow_complex_type &backend=Broker::MEMORY; event dump_tables() diff --git a/testing/btest/broker/store/brokerstore-backend-sqlite-incompatible.zeek b/testing/btest/broker/store/brokerstore-backend-sqlite-incompatible.zeek new file mode 100644 index 0000000000..c895a9c6e8 --- /dev/null +++ b/testing/btest/broker/store/brokerstore-backend-sqlite-incompatible.zeek @@ -0,0 +1,38 @@ +# @TEST-PORT: BROKER_PORT + +# @TEST-EXEC: zeek -B broker -b one.zeek > output1 +# @TEST-EXEC-FAIL: zeek -B broker -b two.zeek > output2 +# @TEST-EXEC: btest-diff .stderr +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr + +# the first test writes out the sqlite files... + +@TEST-START-FILE one.zeek + +module TestModule; + +global t: table[string] of string &backend=Broker::SQLITE; + +event zeek_init() + { + t["a"] = "a"; + t["b"] = "b"; + t["c"] = "c"; + print t; + } + +@TEST-END-FILE +@TEST-START-FILE two.zeek + +# the second one reads them in again. Or not because the types are incompatible. + +module TestModule; + +global t: table[count] of count &backend=Broker::SQLITE; + + +event zeek_init() + { + print t; + } +@TEST-END-FILE diff --git a/testing/btest/broker/store/brokerstore-backend-sqlite.zeek b/testing/btest/broker/store/brokerstore-backend-sqlite.zeek index 0eb728fe22..1ccbc18c20 100644 --- a/testing/btest/broker/store/brokerstore-backend-sqlite.zeek +++ b/testing/btest/broker/store/brokerstore-backend-sqlite.zeek @@ -33,7 +33,7 @@ type testrec: record { global t: table[string] of count &backend=Broker::SQLITE; global s: set[string] &backend=Broker::SQLITE; -global r: table[string] of testrec &backend=Broker::SQLITE; +global r: table[string] of testrec &broker_allow_complex_type &backend=Broker::SQLITE; event zeek_init() { @@ -71,7 +71,7 @@ type testrec: record { global t: table[string] of count &backend=Broker::SQLITE; global s: set[string] &backend=Broker::SQLITE; -global r: table[string] of testrec &backend=Broker::SQLITE; +global r: table[string] of testrec &broker_allow_complex_type &backend=Broker::SQLITE; redef Broker::auto_store_db_directory = ".."; @@ -104,7 +104,7 @@ type testrec: record { global t: table[string] of count &backend=Broker::MEMORY; global s: set[string] &backend=Broker::MEMORY; -global r: table[string] of testrec &backend=Broker::MEMORY; +global r: table[string] of testrec &broker_allow_complex_type &backend=Broker::MEMORY; event dump_tables() From 1888d6acaec2183c3cc3130a4d5dec30726cfdca Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Mon, 13 Jul 2020 17:22:23 -0700 Subject: [PATCH 19/26] BrokerStore <-> Zeek Tables: cleanup and bug workaround --- src/Attr.cc | 4 ++-- src/broker/Manager.cc | 4 +++- .../btest/broker/store/brokerstore-attr-expire.zeek | 10 +++++++++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/Attr.cc b/src/Attr.cc index 9a446fb763..351a360618 100644 --- a/src/Attr.cc +++ b/src/Attr.cc @@ -20,8 +20,8 @@ const char* attr_name(AttrTag t) "&read_expire", "&write_expire", "&create_expire", "&raw_output", "&priority", "&group", "&log", "&error_handler", "&type_column", - "(&tracked)", "&on_change", "&broker_store", - "&broker_allow_complex_type", "&backend", "&deprecated", + "(&tracked)", "&on_change", "&broker_store", + "&broker_allow_complex_type", "&backend", "&deprecated", }; return attr_names[int(t)]; diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index cc689bbf90..4ba4a34aef 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -921,7 +921,9 @@ void Manager::Process() { // Ensure that time gets update before processing broker messages, or events // based on them might get scheduled wrong. - net_update_time(current_time()); + // Fixme: unclear if final solution - see https://github.com/zeek/broker/issues/135 + if ( use_real_time ) + net_update_time(current_time()); bool had_input = false; diff --git a/testing/btest/broker/store/brokerstore-attr-expire.zeek b/testing/btest/broker/store/brokerstore-attr-expire.zeek index be58d3e005..42408fb740 100644 --- a/testing/btest/broker/store/brokerstore-attr-expire.zeek +++ b/testing/btest/broker/store/brokerstore-attr-expire.zeek @@ -1,10 +1,18 @@ +# So - this test currently is not really that great. The goal was to test expiration after +# syncing values with broker. However, it turns out that the delays introduced by broker seem +# a bit random - and too high to really test this without the test taking forever. +# +# so - instead we just check that expiries do indeed happen - however the ordering is not as +# guaranteed as I would have liked to have it. + + # @TEST-PORT: BROKER_PORT # @TEST-EXEC: btest-bg-run master "zeek -B broker -b ../master.zeek >../master.out" # @TEST-EXEC: btest-bg-run clone "zeek -B broker -b ../clone.zeek >../clone.out" # @TEST-EXEC: btest-bg-wait 20 # -# @TEST-EXEC: btest-diff clone.out +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-sort btest-diff clone.out @TEST-START-FILE master.zeek redef exit_only_after_terminate = T; From 6d2aa84952cb93bccf2730432266d5e53ba86d8a Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Fri, 17 Jul 2020 10:56:28 -0700 Subject: [PATCH 20/26] SyncTables: address feedback part 1 - naming (broker and zeek) This commit fixes capitalization issues. --- NEWS | 2 +- scripts/base/frameworks/broker/store.zeek | 10 +++++----- .../base/frameworks/cluster/broker-stores.zeek | 8 ++++---- scripts/base/init-bare.zeek | 2 +- src/Attr.h | 6 +++--- src/Val.cc | 6 +++--- src/Val.h | 8 ++++---- src/broker/Manager.cc | 7 +++---- src/broker/Manager.h | 18 +++++++++--------- src/broker/Store.h | 2 +- .../broker/store/brokerstore-attr-expire.zeek | 2 +- 11 files changed, 35 insertions(+), 36 deletions(-) diff --git a/NEWS b/NEWS index 6297a75c8e..ec2bb7efe8 100644 --- a/NEWS +++ b/NEWS @@ -57,7 +57,7 @@ New Functionality - Broker Store table synchronization (Experimental). - Zeek now supports synchronizing tables/sets across clusters using a backing broker + Zeek now supports synchronizing tables/sets across clusters using a backing Broker store. The same feature also allows persistent storage of data in tables/sets over Zeek restarts. This feature is implemented using the new ```&backend``` attribute. diff --git a/scripts/base/frameworks/broker/store.zeek b/scripts/base/frameworks/broker/store.zeek index 8a9019bb40..23b4cccc4e 100644 --- a/scripts/base/frameworks/broker/store.zeek +++ b/scripts/base/frameworks/broker/store.zeek @@ -25,14 +25,14 @@ export { ## A negative/zero value indicates to never buffer commands. const default_clone_mutation_buffer_interval = 2min &redef; - ## If set to true, the current node is the master node for broker stores - ## backing zeek tables. By default this value will be automatically set to + ## If set to true, the current node is the master node for Broker stores + ## backing Zeek tables. By default this value will be automatically set to ## true in standalone mode, or on the manager node of a cluster. This value ## should not typically be changed manually. const auto_store_master = T &redef; - ## The directory used for storing persistent database files when using brokerstore - ## backed zeek tables. + ## The directory used for storing persistent database files when using Broker store + ## backed Zeek tables. const auto_store_db_directory = "." &redef; ## Whether a data store query could be completed or not. @@ -393,7 +393,7 @@ export { ## d: the communication data. ## ## Returns: The data type associated with the communication data. - ## Note that broker represents records in the same way as + ## Note that Broker represents records in the same way as ## vectors, so there is no "record" type. global data_type: function(d: Broker::Data): Broker::DataType; diff --git a/scripts/base/frameworks/cluster/broker-stores.zeek b/scripts/base/frameworks/cluster/broker-stores.zeek index 1513e6776d..bf84f378e5 100644 --- a/scripts/base/frameworks/cluster/broker-stores.zeek +++ b/scripts/base/frameworks/cluster/broker-stores.zeek @@ -1,9 +1,9 @@ -##! This script deals with the cluster parts of broker backed zeek tables. +##! This script deals with the cluster parts of Broker backed Zeek tables. ##! It makes sure that the master store is set correctly and that clones ##! are automatically created on the non-manager nodes. # Note - this script should become unnecessary in the future, when we just can -# speculatively attach clones. This should be possible once the new ALM broker +# speculatively attach clones. This should be possible once the new ALM Broker # transport becomes available. @load ./main @@ -11,8 +11,8 @@ module Broker; export { - ## Event that is used by the manager to announce the master stores for zeek backed - ## tables that is uses. + ## Event that is used by the manager to announce the master stores for Broker backed + ## tables. global announce_masters: event(masters: set[string]); } diff --git a/scripts/base/init-bare.zeek b/scripts/base/init-bare.zeek index 87c24b6eae..3534f2aac7 100644 --- a/scripts/base/init-bare.zeek +++ b/scripts/base/init-bare.zeek @@ -747,7 +747,7 @@ type script_id: record { enum_constant: bool; ##< True if the identifier is an enum value. option_value: bool; ##< True if the identifier is an option. redefinable: bool; ##< True if the identifier is declared with the :zeek:attr:`&redef` attribute. - broker_backend: bool; ##< True if the identifier has a broker backend defined using the :zeek:attr:`&backend` attribute. + broker_backend: bool; ##< True if the identifier has a Broker backend defined using the :zeek:attr:`&backend` attribute. value: any &optional; ##< The current value of the identifier. }; diff --git a/src/Attr.h b/src/Attr.h index 664a73386f..952bb7a92a 100644 --- a/src/Attr.h +++ b/src/Attr.h @@ -42,9 +42,9 @@ enum AttrTag { ATTR_TYPE_COLUMN, // for input framework ATTR_TRACKED, // hidden attribute, tracked by NotifierRegistry ATTR_ON_CHANGE, // for table change tracking - ATTR_BROKER_STORE, // for broker-store backed tables - ATTR_BROKER_STORE_ALLOW_COMPLEX, // for broker-store backed tables - ATTR_BACKEND, // for broker-store backed tabled + ATTR_BROKER_STORE, // for Broker store backed tables + ATTR_BROKER_STORE_ALLOW_COMPLEX, // for Broker store backed tables + ATTR_BACKEND, // for Broker store backed tabled ATTR_DEPRECATED, NUM_ATTRS // this item should always be last }; diff --git a/src/Val.cc b/src/Val.cc index 133ecad543..f70adf0cc1 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -2164,7 +2164,7 @@ void TableVal::SendToStore(const Val* index, const TableEntryVal* new_entry_val, return; // we either get passed the raw index_val - or a ListVal with exactly one element. - // Since broker does not support ListVals, we have to unoll this in the second case. + // Since Broker does not support ListVals, we have to unoll this in the second case. const Val* index_val; if ( index->GetType()->Tag() == zeek::TYPE_LIST ) { @@ -2221,7 +2221,7 @@ void TableVal::SendToStore(const Val* index, const TableEntryVal* new_entry_val, { if ( ! new_entry_val ) { - zeek::emit_builtin_error("did not receive new value for broker-store send operation"); + zeek::emit_builtin_error("did not receive new value for Broker datastore send operation"); return; } auto new_value = new_entry_val->GetVal().get(); @@ -2239,7 +2239,7 @@ void TableVal::SendToStore(const Val* index, const TableEntryVal* new_entry_val, handle->store.erase(std::move(*broker_index)); break; case ELEMENT_EXPIRED: - // we do nothing here. The broker store does its own expiration - so the element + // we do nothing here. The Broker store does its own expiration - so the element // should expire at about the same time. break; } diff --git a/src/Val.h b/src/Val.h index ef02fbc3f3..40d35cc58c 100644 --- a/src/Val.h +++ b/src/Val.h @@ -785,7 +785,7 @@ public: * @param new_val The value to assign at the index. For a set, this * must be nullptr. * @param broker_forward Controls if the value will be forwarded to attached - * broker stores. + * Broker stores. * @return True if the assignment type-checked. */ bool Assign(ValPtr index, ValPtr new_val, bool broker_forward = true); @@ -799,7 +799,7 @@ public: * @param new_val The value to assign at the index. For a set, this * must be nullptr. * @param broker_forward Controls if the value will be forwarded to attached - * broker stores. + * Broker stores. * @return True if the assignment type-checked. */ bool Assign(ValPtr index, std::unique_ptr k, @@ -926,7 +926,7 @@ public: * Remove an element from the table and return it. * @param index The index to remove. * @param broker_forward Controls if the remove operation will be forwarded to attached - * broker stores. + * Broker stores. * @return The value associated with the index if it exists, else nullptr. * For a sets that don't really contain associated values, a placeholder * value is returned to differentiate it from non-existent index (nullptr), @@ -1024,7 +1024,7 @@ public: static void DoneParsing(); /** - * Sets the name of the broker store that is backing this table. + * Sets the name of the Broker store that is backing this table. * @param store store that is backing this table. */ void SetBrokerStore(const std::string& store) { broker_store = store; } diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index 4ba4a34aef..fa9a14788d 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -235,7 +235,7 @@ void Manager::InitializeBrokerStoreForwarding() id->GetVal()->AsTableVal()->SetBrokerStore(storename); AddForwardedStore(storename, {zeek::NewRef{}, id->GetVal()->AsTableVal()}); - // we only create masters here. For clones, we do all the work of setting up + // We only create masters here. For clones, we do all the work of setting up // the forwarding - but we do not try to initialize the clone. We can only initialize // the clone, once a node has a connection to a master. This is currently done in scriptland // in scripts/base/frameworks/cluster/broker-stores.zeek. Once the ALM transport is ready @@ -921,7 +921,6 @@ void Manager::Process() { // Ensure that time gets update before processing broker messages, or events // based on them might get scheduled wrong. - // Fixme: unclear if final solution - see https://github.com/zeek/broker/issues/135 if ( use_real_time ) net_update_time(current_time()); @@ -1700,14 +1699,14 @@ void Manager::BrokerStoreToZeekTable(const std::string& name, const StoreHandleV auto value = handle->store.get(key); if ( ! value ) { - reporter->Error("Failed to load value for key %s while importing broker store %s to table", to_string(key).c_str(), name.c_str()); + reporter->Error("Failed to load value for key %s while importing Broker store %s to table", to_string(key).c_str(), name.c_str()); continue; } auto zeek_value = data_to_val(*value, table->GetType()->Yield().get()); if ( ! zeek_value ) { - reporter->Error("Could not convert %s to table value while trying to import broker store %s. Aborting import.", to_string(value).c_str(), name.c_str()); + reporter->Error("Could not convert %s to table value while trying to import Broker store %s. Aborting import.", to_string(value).c_str(), name.c_str()); return; } diff --git a/src/broker/Manager.h b/src/broker/Manager.h index 18b6921641..8274d172bb 100644 --- a/src/broker/Manager.h +++ b/src/broker/Manager.h @@ -304,12 +304,12 @@ public: StoreHandleVal* LookupStore(const std::string& name); /** - * Register a zeek table that is associated with a broker store that is backing it. This - * causes all changes that happen to the brokerstore in the future to be applied to the zeek + * Register a Zeek table that is associated with a Broker store that is backing it. This + * causes all changes that happen to the Broker store in the future to be applied to theZzeek * table. - * A single broker store can only be forwarded to a single table. - * @param name name of the broker store - * @param table pointer to the table/set that is being backed + * A single Broker store can only be forwarded to a single table. + * @param name name of the Broker store. + * @param table pointer to the table/set that is being backed. * @return true on success, false if the named store is already being forwarded. */ bool AddForwardedStore(const std::string& name, zeek::IntrusivePtr table); @@ -359,7 +359,7 @@ public: private: void DispatchMessage(const broker::topic& topic, broker::data msg); - // Process events used for broker store backed zeek tables + // Process events used for Broker store backed zeek tables void ProcessStoreEvent(broker::data msg); void ProcessEvent(const broker::topic& topic, broker::zeek::Event ev); bool ProcessLogCreate(broker::zeek::LogCreate lc); @@ -369,11 +369,11 @@ private: void ProcessError(broker::error err); void ProcessStoreResponse(StoreHandleVal*, broker::store::response response); void FlushPendingQueries(); - // Initializes the masters for broker backed zeek tables when using the &backend attribute + // Initializes the masters for Broker backed Zeek tables when using the &backend attribute void InitializeBrokerStoreForwarding(); - // Check if a broker store is associated to a table on the Zeek side. + // Check if a Broker store is associated to a table on the Zeek side. void CheckForwarding(const std::string& name); - // Send the content of a broker store to the backing table. This is typically used + // Send the content of a Broker store to the backing table. This is typically used // when a master/clone is created. void BrokerStoreToZeekTable(const std::string& name, const StoreHandleVal* handle); diff --git a/src/broker/Store.h b/src/broker/Store.h index 4e982b2c86..191a3e7f1f 100644 --- a/src/broker/Store.h +++ b/src/broker/Store.h @@ -49,7 +49,7 @@ inline zeek::RecordValPtr query_result(zeek::RecordValPtr data) /** * Convert an expiry from a double (used to Zeek) to the format required by Broker * @param e: expire interval as double; 0 if no expiry - * @return expire interval in broker format + * @return expire interval in Broker format */ static broker::optional convert_expiry(double e) { diff --git a/testing/btest/broker/store/brokerstore-attr-expire.zeek b/testing/btest/broker/store/brokerstore-attr-expire.zeek index 42408fb740..f43be15deb 100644 --- a/testing/btest/broker/store/brokerstore-attr-expire.zeek +++ b/testing/btest/broker/store/brokerstore-attr-expire.zeek @@ -1,5 +1,5 @@ # So - this test currently is not really that great. The goal was to test expiration after -# syncing values with broker. However, it turns out that the delays introduced by broker seem +# syncing values with Broker. However, it turns out that the delays introduced by Broker seem # a bit random - and too high to really test this without the test taking forever. # # so - instead we just check that expiries do indeed happen - however the ordering is not as From 930a5c8ebd03a8f6bfff6b86b71940c1f2fce092 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Fri, 17 Jul 2020 11:40:59 -0700 Subject: [PATCH 21/26] TableSync: rename auto_store -> table_store --- scripts/base/frameworks/broker/store.zeek | 4 ++-- scripts/base/frameworks/cluster/broker-stores.zeek | 4 ++-- src/broker/Manager.cc | 4 ++-- testing/btest/broker/store/brokerstore-backend-sqlite.zeek | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/scripts/base/frameworks/broker/store.zeek b/scripts/base/frameworks/broker/store.zeek index 23b4cccc4e..2ca7a1f1a8 100644 --- a/scripts/base/frameworks/broker/store.zeek +++ b/scripts/base/frameworks/broker/store.zeek @@ -29,11 +29,11 @@ export { ## backing Zeek tables. By default this value will be automatically set to ## true in standalone mode, or on the manager node of a cluster. This value ## should not typically be changed manually. - const auto_store_master = T &redef; + const table_store_master = T &redef; ## The directory used for storing persistent database files when using Broker store ## backed Zeek tables. - const auto_store_db_directory = "." &redef; + const table_store_db_directory = "." &redef; ## Whether a data store query could be completed or not. type QueryStatus: enum { diff --git a/scripts/base/frameworks/cluster/broker-stores.zeek b/scripts/base/frameworks/cluster/broker-stores.zeek index bf84f378e5..6b73234708 100644 --- a/scripts/base/frameworks/cluster/broker-stores.zeek +++ b/scripts/base/frameworks/cluster/broker-stores.zeek @@ -19,10 +19,10 @@ export { # If we are not the manager - disable automatically generating masters. We will attach # clones instead. @if ( Cluster::is_enabled() && Cluster::local_node_type() != Cluster::MANAGER ) -redef Broker::auto_store_master = F; +redef Broker::table_store_master = F; @endif -@if ( Broker::auto_store_master ) +@if ( Broker::table_store_master ) global broker_backed_ids: set[string]; diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index fa9a14788d..064b231855 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -151,8 +151,8 @@ void Manager::InitPostScript() log_topic_func = get_option("Broker::log_topic")->AsFunc(); log_id_type = zeek::id::find_type("Log::ID")->AsEnumType(); writer_id_type = zeek::id::find_type("Log::Writer")->AsEnumType(); - zeek_table_manager = get_option("Broker::auto_store_master")->AsBool(); - zeek_table_db_directory = get_option("Broker::auto_store_db_directory")->AsString()->CheckString(); + zeek_table_manager = get_option("Broker::table_store_master")->AsBool(); + zeek_table_db_directory = get_option("Broker::table_store_db_directory")->AsString()->CheckString(); opaque_of_data_type = zeek::make_intrusive("Broker::Data"); opaque_of_set_iterator = zeek::make_intrusive("Broker::SetIterator"); diff --git a/testing/btest/broker/store/brokerstore-backend-sqlite.zeek b/testing/btest/broker/store/brokerstore-backend-sqlite.zeek index 1ccbc18c20..354a9e21d3 100644 --- a/testing/btest/broker/store/brokerstore-backend-sqlite.zeek +++ b/testing/btest/broker/store/brokerstore-backend-sqlite.zeek @@ -73,7 +73,7 @@ global t: table[string] of count &backend=Broker::SQLITE; global s: set[string] &backend=Broker::SQLITE; global r: table[string] of testrec &broker_allow_complex_type &backend=Broker::SQLITE; -redef Broker::auto_store_db_directory = ".."; +redef Broker::table_store_db_directory = ".."; event zeek_init() { From 36db9d8369cbe1b6795aa209f046a1bda16b99c1 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Fri, 17 Jul 2020 14:21:27 -0700 Subject: [PATCH 22/26] TableSync: don't raise &on_change, smaller fixes This addresses PR feedback. The main component in this commit is to disable &on_change notifications when &backend loads a table from sqlite on startup. --- NEWS | 3 +- src/Attr.cc | 5 +-- src/Attr.h | 2 +- src/Val.h | 10 ++++++ src/broker/Manager.cc | 31 +++++++++++-------- src/broker/Manager.h | 2 +- src/broker/Store.h | 2 +- .../store/brokerstore-backend-sqlite.zeek | 8 ++++- 8 files changed, 43 insertions(+), 20 deletions(-) diff --git a/NEWS b/NEWS index ec2bb7efe8..fd246b46b3 100644 --- a/NEWS +++ b/NEWS @@ -65,7 +65,8 @@ New Functionality ```global t: table[string] of count &backend=Broker::MEMORY;``` - This feature is documented in detail here: FIXME. + This feature is documented in detail here: + https://docs.zeek.org/en/current/frameworks/broker.html#broker-store-backed-zeek-tables-for-data-synchronization-and-persistence Note: this feature is experimental and the syntax/featureset can change in the future. diff --git a/src/Attr.cc b/src/Attr.cc index 351a360618..66a084c08e 100644 --- a/src/Attr.cc +++ b/src/Attr.cc @@ -465,7 +465,7 @@ void Attributes::CheckAttr(Attr* a) } if ( Find(ATTR_BACKEND) ) { - Error("&broker_store and &backend cannot be used simultaneously"); + Error("&backend and &broker_store cannot be used simultaneously"); } } // fallthrough @@ -646,7 +646,7 @@ void Attributes::CheckAttr(Attr* a) Error("&backend only supports one-element set/table indexes"); } - // Only support atomic types for the moment. + // Only support atomic types for the moment, unless explicitly overriden if ( ! type->AsTableType()->IsSet() && ! input::Manager::IsCompatibleType(type->AsTableType()->Yield().get(), true) && ! Find(ATTR_BROKER_STORE_ALLOW_COMPLEX) ) @@ -689,6 +689,7 @@ void Attributes::CheckAttr(Attr* a) Error("&broker_store only supports one-element set/table indexes"); } + // Only support atomic types for the moment, unless explicitly overriden if ( ! type->AsTableType()->IsSet() && ! input::Manager::IsCompatibleType(type->AsTableType()->Yield().get(), true) && ! Find(ATTR_BROKER_STORE_ALLOW_COMPLEX) ) diff --git a/src/Attr.h b/src/Attr.h index 952bb7a92a..0e18f0ace2 100644 --- a/src/Attr.h +++ b/src/Attr.h @@ -44,7 +44,7 @@ enum AttrTag { ATTR_ON_CHANGE, // for table change tracking ATTR_BROKER_STORE, // for Broker store backed tables ATTR_BROKER_STORE_ALLOW_COMPLEX, // for Broker store backed tables - ATTR_BACKEND, // for Broker store backed tabled + ATTR_BACKEND, // for Broker store backed tables ATTR_DEPRECATED, NUM_ATTRS // this item should always be last }; diff --git a/src/Val.h b/src/Val.h index 40d35cc58c..8000c5ed06 100644 --- a/src/Val.h +++ b/src/Val.h @@ -1029,6 +1029,16 @@ public: */ void SetBrokerStore(const std::string& store) { broker_store = store; } + /** + * Disable change notification processing of &on_change until re-enabled. + */ + void DisableChangeNotifications() { in_change_func = true; } + + /** + * Re-enables change notifcations after being disabled by DisableChangeNotifications. + */ + void EnableChangeNotifications() { in_change_func = false; } + protected: void Init(zeek::TableTypePtr t); diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index 064b231855..49a1728a6c 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -1009,13 +1009,13 @@ void Manager::ProcessStoreEvent(broker::data msg) if ( ! table ) return; - auto key = insert.key(); - auto data = insert.value(); - // We sent this message. Ignore it. if ( insert.publisher() == storehandle->store_pid ) return; + auto key = insert.key(); + auto data = insert.value(); + DBG_LOG(DBG_BROKER, "Store %s: Insert: %s:%s (%s:%s)", insert.store_id().c_str(), to_string(insert.key()).c_str(), to_string(insert.value()).c_str(), insert.key().get_type_name(), insert.value().get_type_name()); if ( table->GetType()->IsSet() && data.get_type() != broker::data::type::none ) @@ -1058,13 +1058,13 @@ void Manager::ProcessStoreEvent(broker::data msg) if ( ! table ) return; - auto key = update.key(); - auto data = update.new_value(); - // We sent this message. Ignore it. if ( update.publisher() == storehandle->store_pid ) return; + auto key = update.key(); + auto data = update.new_value(); + DBG_LOG(DBG_BROKER, "Store %s: Update: %s->%s (%s)", update.store_id().c_str(), to_string(update.old_value()).c_str(), to_string(update.new_value()).c_str(), update.new_value().get_type_name()); if ( table->GetType()->IsSet() && data.get_type() != broker::data::type::none ) @@ -1653,7 +1653,7 @@ StoreHandleVal* Manager::MakeMaster(const string& name, broker::backend type, data_stores.emplace(name, handle); iosource_mgr->RegisterFd(handle->proxy.mailbox().descriptor(), this); - CheckForwarding(name); + PrepareForwarding(name); if ( ! bstate->endpoint.use_real_time() ) // Wait for master to become available/responsive. @@ -1667,7 +1667,6 @@ StoreHandleVal* Manager::MakeMaster(const string& name, broker::backend type, void Manager::BrokerStoreToZeekTable(const std::string& name, const StoreHandleVal* handle) { - // consider if it might be wise to disable &on_change while filling the table if ( ! handle->forward_to ) return; @@ -1680,6 +1679,10 @@ void Manager::BrokerStoreToZeekTable(const std::string& name, const StoreHandleV const auto& its = table->GetType()->AsTableType()->GetIndexTypes(); bool is_set = table->GetType()->IsSet(); assert( its.size() == 1 ); + + // disable &on_change notifications while filling the table. + table->DisableChangeNotifications(); + for ( const auto key : *set ) { auto zeek_key = data_to_val(key, its[0].get()); @@ -1687,6 +1690,7 @@ void Manager::BrokerStoreToZeekTable(const std::string& name, const StoreHandleV { reporter->Error("Failed to convert key \"%s\" while importing broker store to table for store \"%s\". Aborting import.", to_string(key).c_str(), name.c_str()); // just abort - this probably means the types are incompatible + table->EnableChangeNotifications(); return; } @@ -1700,6 +1704,7 @@ void Manager::BrokerStoreToZeekTable(const std::string& name, const StoreHandleV if ( ! value ) { reporter->Error("Failed to load value for key %s while importing Broker store %s to table", to_string(key).c_str(), name.c_str()); + table->EnableChangeNotifications(); continue; } @@ -1707,11 +1712,13 @@ void Manager::BrokerStoreToZeekTable(const std::string& name, const StoreHandleV if ( ! zeek_value ) { reporter->Error("Could not convert %s to table value while trying to import Broker store %s. Aborting import.", to_string(value).c_str(), name.c_str()); + table->EnableChangeNotifications(); return; } table->Assign(zeek_key, zeek_value, false); } + table->EnableChangeNotifications(); return; } @@ -1742,7 +1749,7 @@ StoreHandleVal* Manager::MakeClone(const string& name, double resync_interval, data_stores.emplace(name, handle); iosource_mgr->RegisterFd(handle->proxy.mailbox().descriptor(), this); - CheckForwarding(name); + PrepareForwarding(name); return handle; } @@ -1760,8 +1767,6 @@ bool Manager::CloseStore(const string& name) if ( s == data_stores.end() ) return false; - auto pubid = s->second->store.frontend_id(); - iosource_mgr->UnregisterFd(s->second->proxy.mailbox().descriptor(), this); for ( auto i = pending_queries.begin(); i != pending_queries.end(); ) @@ -1815,11 +1820,11 @@ bool Manager::AddForwardedStore(const std::string& name, zeek::IntrusivePtr Date: Fri, 17 Jul 2020 15:27:01 -0700 Subject: [PATCH 23/26] TableSync: refactor common functionality into function This addresses feedback and puts the common update and insert functionality into its own function. --- src/broker/Manager.cc | 115 +++++++----------- src/broker/Manager.h | 2 + .../worker-1..stderr | 4 +- 3 files changed, 50 insertions(+), 71 deletions(-) diff --git a/src/broker/Manager.cc b/src/broker/Manager.cc index 49a1728a6c..a1b7c7d530 100644 --- a/src/broker/Manager.cc +++ b/src/broker/Manager.cc @@ -997,6 +997,49 @@ void Manager::Process() } } +void Manager::ProcessStoreEventInsertUpdate(zeek::IntrusivePtr table, const std::string& store_id, const broker::data& key, const broker::data& data, const broker::data& old_value, bool insert) + { + auto type = "Insert"; + if ( ! insert ) + type = "Update"; + + if ( insert ) + DBG_LOG(DBG_BROKER, "Store %s: Insert: %s:%s (%s:%s)", store_id.c_str(), to_string(key).c_str(), to_string(data).c_str(), key.get_type_name(), data.get_type_name()); + else + DBG_LOG(DBG_BROKER, "Store %s: Update: %s->%s (%s)", store_id.c_str(), to_string(old_value).c_str(), to_string(data).c_str(), data.get_type_name()); + + + if ( table->GetType()->IsSet() && data.get_type() != broker::data::type::none ) + { + reporter->Error("ProcessStoreEvent %s got %s when expecting set", type, data.get_type_name()); + return; + } + + const auto& its = table->GetType()->AsTableType()->GetIndexTypes(); + assert( its.size() == 1 ); + auto zeek_key = data_to_val(key, its[0].get()); + if ( ! zeek_key ) + { + reporter->Error("ProcessStoreEvent %s: could not convert key \"%s\" for store \"%s\" while receiving remote data. This probably means the tables have different types on different nodes.", type, to_string(key).c_str(), store_id.c_str()); + return; + } + + if ( table->GetType()->IsSet() ) + { + table->Assign(zeek_key, nullptr, false); + return; + } + + // it is a table + auto zeek_value = data_to_val(data, table->GetType()->Yield().get()); + if ( ! zeek_value ) + { + reporter->Error("ProcessStoreEvent %s: could not convert value \"%s\" for key \"%s\" in store \"%s\" while receiving remote data. This probably means the tables have different types on different nodes.", type, to_string(data).c_str(), to_string(key).c_str(), store_id.c_str()); + return; + } + table->Assign(zeek_key, zeek_value, false); + } + void Manager::ProcessStoreEvent(broker::data msg) { if ( auto insert = broker::store_event::insert::make(msg) ) @@ -1013,41 +1056,8 @@ void Manager::ProcessStoreEvent(broker::data msg) if ( insert.publisher() == storehandle->store_pid ) return; - auto key = insert.key(); - auto data = insert.value(); - - DBG_LOG(DBG_BROKER, "Store %s: Insert: %s:%s (%s:%s)", insert.store_id().c_str(), to_string(insert.key()).c_str(), to_string(insert.value()).c_str(), insert.key().get_type_name(), insert.value().get_type_name()); - - if ( table->GetType()->IsSet() && data.get_type() != broker::data::type::none ) - { - reporter->Error("ProcessStoreEvent Insert got %s when expecting set", data.get_type_name()); - return; - } - - const auto& its = table->GetType()->AsTableType()->GetIndexTypes(); - assert( its.size() == 1 ); - auto zeek_key = data_to_val(key, its[0].get()); - if ( ! zeek_key ) - { - reporter->Error("ProcessStoreEvent: could not convert key \"%s\" for store \"%s\" while receiving remote insert. This probably means the tables have different types on different nodes.", to_string(key).c_str(), insert.store_id().c_str()); - return; - } - - if ( table->GetType()->IsSet() ) - { - table->Assign(zeek_key, nullptr, false); - return; - } - - // it is a table - auto zeek_value = data_to_val(data, table->GetType()->Yield().get()); - if ( ! zeek_value ) - { - reporter->Error("ProcessStoreEvent: could not convert value \"%s\" for key \"%s\" in store \"%s\" while receiving remote insert. This probably means the tables have different types on different nodes.", to_string(data).c_str(), to_string(key).c_str(), insert.store_id().c_str()); - return; - } - table->Assign(zeek_key, zeek_value, false); - } + ProcessStoreEventInsertUpdate(table, insert.store_id(), insert.key(), insert.value(), {}, true); + } else if ( auto update = broker::store_event::update::make(msg) ) { auto storehandle = broker_mgr->LookupStore(update.store_id()); @@ -1062,40 +1072,7 @@ void Manager::ProcessStoreEvent(broker::data msg) if ( update.publisher() == storehandle->store_pid ) return; - auto key = update.key(); - auto data = update.new_value(); - - DBG_LOG(DBG_BROKER, "Store %s: Update: %s->%s (%s)", update.store_id().c_str(), to_string(update.old_value()).c_str(), to_string(update.new_value()).c_str(), update.new_value().get_type_name()); - - if ( table->GetType()->IsSet() && data.get_type() != broker::data::type::none ) - { - reporter->Error("ProcessStoreEvent Update got %s when expecting set", data.get_type_name()); - return; - } - - const auto& its = table->GetType()->AsTableType()->GetIndexTypes(); - assert( its.size() == 1 ); - auto zeek_key = data_to_val(key, its[0].get()); - if ( ! zeek_key ) - { - reporter->Error("ProcessStoreEvent: could not convert key \"%s\" for store \"%s\" while receiving remote update. This probably means the tables have different types on different nodes.", to_string(key).c_str(), insert.store_id().c_str()); - return; - } - - if ( table->GetType()->IsSet() ) - { - table->Assign(zeek_key, nullptr, false); - return; - } - - // it is a table - auto zeek_value = data_to_val(data, table->GetType()->Yield().get()); - if ( ! zeek_value ) - { - reporter->Error("ProcessStoreEvent: could not convert value \"%s\" for key \"%s\" in store \"%s\" while receiving remote update. This probably means the tables have different types on different nodes.", to_string(data).c_str(), to_string(key).c_str(), insert.store_id().c_str()); - return; - } - table->Assign(zeek_key, zeek_value, false); + ProcessStoreEventInsertUpdate(table, update.store_id(), update.key(), update.new_value(), update.old_value(), false); } else if ( auto erase = broker::store_event::erase::make(msg) ) { diff --git a/src/broker/Manager.h b/src/broker/Manager.h index 9b5af56183..bcd231aa9e 100644 --- a/src/broker/Manager.h +++ b/src/broker/Manager.h @@ -361,6 +361,8 @@ private: void DispatchMessage(const broker::topic& topic, broker::data msg); // Process events used for Broker store backed zeek tables void ProcessStoreEvent(broker::data msg); + // Common functionality for processing insert and update events. + void ProcessStoreEventInsertUpdate(zeek::IntrusivePtr table, const std::string& store_id, const broker::data& key, const broker::data& data, const broker::data& old_value, bool insert); void ProcessEvent(const broker::topic& topic, broker::zeek::Event ev); bool ProcessLogCreate(broker::zeek::LogCreate lc); bool ProcessLogWrite(broker::zeek::LogWrite lw); diff --git a/testing/btest/Baseline/broker.store.brokerstore-backend-simple-incompatible/worker-1..stderr b/testing/btest/Baseline/broker.store.brokerstore-backend-simple-incompatible/worker-1..stderr index 8a5d27c280..ea16941541 100644 --- a/testing/btest/Baseline/broker.store.brokerstore-backend-simple-incompatible/worker-1..stderr +++ b/testing/btest/Baseline/broker.store.brokerstore-backend-simple-incompatible/worker-1..stderr @@ -1,3 +1,3 @@ -error: ProcessStoreEvent: could not convert value "b" for key "a" in store "___sync_store_TestModule::s" while receiving remote insert. This probably means the tables have different types on different nodes. -error: ProcessStoreEvent: could not convert key "a" for store "___sync_store_TestModule::t" while receiving remote insert. This probably means the tables have different types on different nodes. +error: ProcessStoreEvent Insert: could not convert value "b" for key "a" in store "___sync_store_TestModule::s" while receiving remote data. This probably means the tables have different types on different nodes. +error: ProcessStoreEvent Insert: could not convert key "a" for store "___sync_store_TestModule::t" while receiving remote data. This probably means the tables have different types on different nodes. received termination signal From 42b566935e4041c11ff51dc481525766c4142725 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Fri, 17 Jul 2020 16:26:02 -0700 Subject: [PATCH 24/26] Try to make FreeBSD test happy with larger timeout. --- .../btest/broker/store/brokerstore-backend-simple-reverse.zeek | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/btest/broker/store/brokerstore-backend-simple-reverse.zeek b/testing/btest/broker/store/brokerstore-backend-simple-reverse.zeek index 8ec17e5f32..62a76d9246 100644 --- a/testing/btest/broker/store/brokerstore-backend-simple-reverse.zeek +++ b/testing/btest/broker/store/brokerstore-backend-simple-reverse.zeek @@ -5,7 +5,7 @@ # @TEST-EXEC: btest-bg-run manager-1 "ZEEKPATH=$ZEEKPATH:.. CLUSTER_NODE=manager-1 zeek -B broker ../master.zeek >../master.out" # @TEST-EXEC: btest-bg-run worker-1 "ZEEKPATH=$ZEEKPATH:.. CLUSTER_NODE=worker-1 zeek -B broker ../clone.zeek >../clone.out" # @TEST-EXEC: btest-bg-run worker-2 "ZEEKPATH=$ZEEKPATH:.. CLUSTER_NODE=worker-2 zeek -B broker ../clone2.zeek >../clone2.out" -# @TEST-EXEC: btest-bg-wait 15 +# @TEST-EXEC: btest-bg-wait 30 # # @TEST-EXEC: btest-diff master.out # @TEST-EXEC: btest-diff clone.out From 095491711ea244855e42b4b39edc7a69835d74ee Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Fri, 17 Jul 2020 17:14:44 -0700 Subject: [PATCH 25/26] Increase timeouts to see if FreeBSD will be happy with this. --- .../broker/store/brokerstore-backend-simple-reverse.zeek | 8 ++++---- .../btest/broker/store/brokerstore-backend-sqlite.zeek | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/testing/btest/broker/store/brokerstore-backend-simple-reverse.zeek b/testing/btest/broker/store/brokerstore-backend-simple-reverse.zeek index 62a76d9246..545465dbf6 100644 --- a/testing/btest/broker/store/brokerstore-backend-simple-reverse.zeek +++ b/testing/btest/broker/store/brokerstore-backend-simple-reverse.zeek @@ -5,7 +5,7 @@ # @TEST-EXEC: btest-bg-run manager-1 "ZEEKPATH=$ZEEKPATH:.. CLUSTER_NODE=manager-1 zeek -B broker ../master.zeek >../master.out" # @TEST-EXEC: btest-bg-run worker-1 "ZEEKPATH=$ZEEKPATH:.. CLUSTER_NODE=worker-1 zeek -B broker ../clone.zeek >../clone.out" # @TEST-EXEC: btest-bg-run worker-2 "ZEEKPATH=$ZEEKPATH:.. CLUSTER_NODE=worker-2 zeek -B broker ../clone2.zeek >../clone2.out" -# @TEST-EXEC: btest-bg-wait 30 +# @TEST-EXEC: btest-bg-wait 40 # # @TEST-EXEC: btest-diff master.out # @TEST-EXEC: btest-diff clone.out @@ -91,13 +91,13 @@ event dump_tables() print t; print s; print r; - schedule 2sec { terminate_me() }; + schedule 10sec { terminate_me() }; } event Cluster::node_up(name: string, id: string) { #print "node up", name; - schedule 1secs { dump_tables() }; + schedule 5secs { dump_tables() }; } @TEST-END-FILE @@ -129,7 +129,7 @@ event dump_tables() event Cluster::node_up(name: string, id: string) { #print "node up", name; - schedule 4secs { dump_tables() }; + schedule 20secs { dump_tables() }; } @TEST-END-FILE diff --git a/testing/btest/broker/store/brokerstore-backend-sqlite.zeek b/testing/btest/broker/store/brokerstore-backend-sqlite.zeek index 10bc03c58f..eb34a51379 100644 --- a/testing/btest/broker/store/brokerstore-backend-sqlite.zeek +++ b/testing/btest/broker/store/brokerstore-backend-sqlite.zeek @@ -6,7 +6,7 @@ # @TEST-EXEC: btest-bg-run manager-1 "ZEEKPATH=$ZEEKPATH:.. CLUSTER_NODE=manager-1 zeek -B broker ../master.zeek >../master.out" # @TEST-EXEC: btest-bg-run worker-1 "ZEEKPATH=$ZEEKPATH:.. CLUSTER_NODE=worker-1 zeek -B broker ../clone.zeek >../clone.out" # @TEST-EXEC: btest-bg-run worker-2 "ZEEKPATH=$ZEEKPATH:.. CLUSTER_NODE=worker-2 zeek -B broker ../clone.zeek >../clone2.out" -# @TEST-EXEC: btest-bg-wait 15 +# @TEST-EXEC: btest-bg-wait 40 # # @TEST-EXEC: btest-diff master.out # @TEST-EXEC: btest-diff clone.out @@ -124,6 +124,6 @@ event dump_tables() event Cluster::node_up(name: string, id: string) { #print "node up", name; - schedule 4secs { dump_tables() }; + schedule 15secs { dump_tables() }; } @TEST-END-FILE From a505ed4bfe81ff55204776a666d9e579801b5652 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Fri, 17 Jul 2020 18:59:52 -0700 Subject: [PATCH 26/26] TableSync: try to make test more robust & add debug output --- .../brokerstore-backend-simple-reverse.zeek | 35 +++++++++++++++---- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/testing/btest/broker/store/brokerstore-backend-simple-reverse.zeek b/testing/btest/broker/store/brokerstore-backend-simple-reverse.zeek index 545465dbf6..c44f66f597 100644 --- a/testing/btest/broker/store/brokerstore-backend-simple-reverse.zeek +++ b/testing/btest/broker/store/brokerstore-backend-simple-reverse.zeek @@ -38,16 +38,28 @@ global t: table[string] of count &backend=Broker::MEMORY; global s: set[string] &backend=Broker::MEMORY; global r: table[string] of testrec &broker_allow_complex_type &backend=Broker::MEMORY; +global terminate_count = 0; + event zeek_init() { } +event Broker::peer_added(endpoint: Broker::EndpointInfo, msg: string) &priority=1 + { + Reporter::info(fmt("Peer added: %s", cat(endpoint))); + } + event Broker::peer_lost(endpoint: Broker::EndpointInfo, msg: string) { - print t; - print s; - print r; - terminate(); + Reporter::info(fmt("Peer lost: %s", cat(endpoint))); + terminate_count += 1; + if ( terminate_count == 2) + { + terminate(); + print t; + print s; + print r; + } } @TEST-END-FILE @@ -96,9 +108,15 @@ event dump_tables() event Cluster::node_up(name: string, id: string) { - #print "node up", name; + Reporter::info(fmt("Node Up: %s", name)); schedule 5secs { dump_tables() }; } + +event Broker::announce_masters(masters: set[string]) + { + Reporter::info(fmt("Received announce_masters: %s", cat(masters))); + } + @TEST-END-FILE @TEST-START-FILE clone2.zeek @@ -126,9 +144,14 @@ event dump_tables() terminate(); } +event Broker::announce_masters(masters: set[string]) + { + Reporter::info(fmt("Received announce_masters: %s", cat(masters))); + } + event Cluster::node_up(name: string, id: string) { - #print "node up", name; + Reporter::info(fmt("Node Up: %s", name)); schedule 20secs { dump_tables() }; } @TEST-END-FILE