Time consistency #86

aliciayuting · 2026-01-14T00:23:45Z

Implementation of time consistent put_by_time and get_by_time, and their test cases.

Adding an API for put_by_time that rejects if ts < now - \delta (\delta is the clock skew between servers)

etremel

This looks pretty good. There aren't too many server-side changes needed, since the version Cascade puts in object headers in internal_ordered_put will be the version in the Derecho message header that triggered the ordered_put, so just changing to ordered_send_with_timestamp should set both timestamps to client-provided values.

My comments are mostly about your test cases, and I have one overall question about them: Why are they in the cascade_as_subgroup_classes directory? I thought this was for demonstrating how you could customize Cascade with your own client and server code, instead of using the standard "Cascade service." Do your tests expect to use non-standard server code? Or do you intend to run them against the standard Cascade service (i.e. src/service/server.cpp), and you just have custom client code? In the latter case, they should probably go in a different folder, although I'm not quite sure which one. The unit_tests folder seems to be for tests that don't even require launching the servers, while the other two seem to be for tests that use UDLs on the standard Cascade service.

etremel · 2026-01-15T17:50:51Z

include/cascade/detail/trigger_store_impl.hpp

+template <typename KT, typename VT, KT* IK, VT* IV>
+version_tuple TriggerCascadeNoStore<KT, VT, IK, IV>::put_with_timestamp(const VT& value, uint64_t timestamp_us, bool as_trigger) const {
+    // TriggerCascadeNoStore doesn't support timestamps, fall back to put_and_forget
+    put_and_forget(value, as_trigger);


Why call put_and_forget? Why not just return an error the same way put() does? Any put other than trigger_put is not supported on TriggerCascadeNoStore.

Good point. It should be error. Edited in the new commit.

etremel · 2026-01-15T18:01:41Z

src/applications/tests/cascade_as_subgroup_classes/CMakeLists.txt

+        ${CMAKE_CURRENT_BINARY_DIR}/consistency_test_cfg/n2/udl_dlls.cfg
+    COMMAND ln -sf ${CMAKE_CURRENT_BINARY_DIR}/consistency_test_cfg/udl_dlls.cfg.tmp
+        ${CMAKE_CURRENT_BINARY_DIR}/consistency_test_cfg/n3/udl_dlls.cfg
+    DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/consistency_test_cfg/run.sh.tmp


This command says that the consistency_test_cfg folder should contain a file named run.sh.tmp, but this file is not in the pull request.

etremel · 2026-01-15T18:18:28Z

src/applications/tests/cascade_as_subgroup_classes/test_put_by_time.cpp

+            uint64_t future_time_us = current_time_us + 500000ULL; // 500ms = 500000 microseconds
+            std::cout << "  Current time: " << current_time_us << " microseconds" << std::endl;
+            std::cout << "  Future time: " << future_time_us << " microseconds" << std::endl;
+            std::cout << "  Calling put_by_time..." << std::endl;


What if it takes almost 500ms to print these messages? Terminal output can be slow, especially when measuring precise timing. To make sure the timestamp is in the future by the time you send it, you might want to wait to print anything until after you call put_by_time.

Great catch. Edited in the new commit

etremel · 2026-01-15T18:20:04Z

src/applications/tests/cascade_as_subgroup_classes/test_put_by_time.cpp

+            // (regular_put_timestamp + 100ms to be safe)
+            uint64_t custom_timestamp_us = regular_put_timestamp + 100000ULL; // 100ms ahead of regular put
+            std::cout << "  put_by_time with custom timestamp: " << custom_timestamp_us << " microseconds" << std::endl;
+            std::cout << "    (100ms ahead of regular put to ensure HLC advances)" << std::endl;


Same concern about the time it takes to print these messages.

etremel · 2026-01-15T18:26:10Z

src/applications/tests/cascade_as_subgroup_classes/consistency_test_cfg/n0/derecho.cfg

@@ -0,0 +1,197 @@
+[DERECHO]
+# leader ip - the leader's ip address


Configuration files for test cases don't need all these documentation comments, unless we expect outside users to read and run these tests.

etremel · 2026-01-15T18:34:08Z

src/applications/tests/cascade_as_subgroup_classes/consistency_test_cfg/n0/derecho.cfg

+# leader gms port - the leader's gms port
+contact_port = 23580
+# my local id - each node should have a different id
+local_id = 0


Remember that in the Derecho 2.4 config system, you can put these node-specific options in a separate derecho_node.cfg file, and then make derecho.cfg exactly the same for all nodes.

Nice. I tried this one in the new commit. Very convinient!

etremel · 2026-01-15T18:39:47Z

src/applications/tests/cascade_as_subgroup_classes/consistency_test_cfg/udl_dlls.cfg.tmp

@@ -0,0 +1 @@
+../../libconsole_printer_udl.so


Do your tests actually need the server to run the console_printer_udl, or was this copied over from another test inadvertently?

Thank you for the catch. This was copied over from another test inadvertently. I will fix this in the new commit.

etremel · 2026-01-15T18:49:42Z

include/cascade/detail/service_client_impl.hpp

+        const ObjectType& value, const uint64_t& timestamp_us, bool as_trigger) {
+    // STEP 1 - get key
+    if constexpr (!std::is_base_of_v<ICascadeObject<std::string,ObjectType>,ObjectType>) {
+        throw derecho::derecho_exception(std::string("ServiceClient<>::put_by_time() only support object of type ICascadeObject<std::string,ObjectType>,but we get ") + typeid(ObjectType).name());


You might want to fix the grammar in this error message while you're copying it. It should be "only supports objects of type... but got"

etremel · 2026-01-15T22:12:27Z

include/cascade/detail/persistent_store_impl.hpp

 }

+template <typename KT, typename VT, KT* IK, VT* IV, persistent::StorageType ST>
+version_tuple PersistentCascadeStore<KT, VT, IK, IV, ST>::put_with_timestamp(const VT& value, uint64_t timestamp_us, bool as_trigger) const {


To be safe, the server should reject the message here if the timestamp is too old. (The client's put_by_time function shouldn't send the message with an old timestamp in the first place, but someone might write a nonstandard client, and other server functions are similarly defensive in rejecting invalid arguments).

…ode(currently in comments, need discussion of the exact value to check)

aliciayuting added 2 commits January 12, 2026 19:18

implemented put_by_time

9587c4a

get_by_time implementation and test cases

b93eab7

aliciayuting requested a review from etremel January 14, 2026 00:23

added logic for RPC function of put_by_time for external client

3060041

etremel requested changes Jan 15, 2026

View reviewed changes

etremel reviewed Jan 15, 2026

View reviewed changes

aliciayuting added 2 commits January 18, 2026 20:03

fixes for the temporal consistency test cases, added p2p validation c…

7f7bae3

…ode(currently in comments, need discussion of the exact value to check)

added Epsilon to account for client to server delay

e52a966

		@@ -0,0 +1,197 @@
		[DERECHO]
		# leader ip - the leader's ip address

		@@ -0,0 +1 @@
		../../libconsole_printer_udl.so

Time consistency #86

Are you sure you want to change the base?

Time consistency #86

Uh oh!

Conversation

aliciayuting commented Jan 14, 2026

Uh oh!

etremel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants