Skip to content

Commit

Permalink
fix(storage-manager): do not start when 'timestamping' is disabled (#…
Browse files Browse the repository at this point in the history
…1219)

All storage must have a timestamp associated with a Sample.

As it is possible to publish without adding a timestamp, it means that a
Zenoh node must add this timestamp "at some point". Up until now, the
default configuration of a router ('timestamping' enabled) combined with
the fact that only routers could load plugins (and, thus, storage) made
it so that a timestamp was (by default) always added.

Recent changes in Zenoh — namely the fact that not only routers can load
plugins and that peers and client have, by default, the 'timestamping'
configuration disabled — invalidate these assumptions.

We should then enforce at runtime, that the 'timestamping' configuration
is enabled when attempting to load the storage manager.

This commit adds this check by verifying that there is an HLC associated
with the Zenoh Session — the HLC is only created if 'timestamping' is
enabled (see `zenoh/zenoh/src/net/runtime/mod.rs::142`).

* plugins/zenoh-plugin-storage-manager/src/lib.rs: return an error if
  the storage manager is started while the configuration option
  'timestamping' is disabled.
* plugins/zenoh-plugin-storage-manager/tests/operations.rs: updated
  the `config` used in the test to enable 'timestamping'.
* plugins/zenoh-plugin-storage-manager/tests/wildcard.rs: updated the
  `config` used in the test to enable 'timestamping'.

Signed-off-by: Julien Loudet <[email protected]>
  • Loading branch information
J-Loudet authored Jul 7, 2024
1 parent 418b5a6 commit 6df74c7
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 0 deletions.
17 changes: 17 additions & 0 deletions plugins/zenoh-plugin-storage-manager/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use memory_backend::MemoryBackend;
use storages_mgt::StorageMessage;
use zenoh::{
internal::{
bail,
plugins::{Response, RunningPlugin, RunningPluginTrait, ZenohPlugin},
runtime::Runtime,
zlock, LibLoader,
Expand Down Expand Up @@ -120,6 +121,22 @@ impl StorageRuntimeInner {

let session = Arc::new(zenoh::session::init(runtime.clone()).wait()?);

// NOTE: All storage **must** have a timestamp associated with a Sample. Considering that it is possible to make
// a publication without associating a timestamp, that means that the node managing the storage (be it a
// Zenoh client / peer / router) has to add it.
//
// If the `timestamping` configuration setting is disabled then there is no HLC associated with the
// Session. That eventually means that no timestamp can be generated which goes against the previous
// requirement.
//
// Hence, in that scenario, we refuse to start the storage manager and any storage.
if session.hlc().is_none() {
tracing::error!(
"Cannot start storage manager (and thus any storage) without the 'timestamping' setting enabled in the Zenoh configuration"
);
bail!("Cannot start storage manager, 'timestamping' is disabled in the configuration");
}

// After this moment result should be only Ok. Failure of loading of one voulme or storage should not affect others.

let mut new_self = StorageRuntimeInner {
Expand Down
12 changes: 12 additions & 0 deletions plugins/zenoh-plugin-storage-manager/tests/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,18 @@ async fn test_updates_in_order() {
}"#,
)
.unwrap();
config
.insert_json5(
"timestamping",
r#"{
enabled: {
router: true,
peer: true,
client: true
}
}"#,
)
.unwrap();

let runtime = zenoh::internal::runtime::RuntimeBuilder::new(config)
.build()
Expand Down
12 changes: 12 additions & 0 deletions plugins/zenoh-plugin-storage-manager/tests/wildcard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,18 @@ async fn test_wild_card_in_order() {
}"#,
)
.unwrap();
config
.insert_json5(
"timestamping",
r#"{
enabled: {
router: true,
peer: true,
client: true
}
}"#,
)
.unwrap();

let runtime = zenoh::internal::runtime::RuntimeBuilder::new(config)
.build()
Expand Down

0 comments on commit 6df74c7

Please sign in to comment.