diff --git a/Makefile b/Makefile index 42caf79f..4303b97b 100644 --- a/Makefile +++ b/Makefile @@ -58,7 +58,6 @@ SCYLLA_TEST_FILTER := $(subst ${SPACE},${EMPTY},ClusterTests.*\ :-SchemaMetadataTest.Integration_Cassandra_RegularMetadataNotMarkedVirtual\ :SchemaMetadataTest.Integration_Cassandra_VirtualMetadata\ :HeartbeatTests.Integration_Cassandra_HeartbeatFailed\ -:TimestampTests.Integration_Cassandra_MonotonicTimestampGenerator\ :ExecutionProfileTest.Integration_Cassandra_RoundRobin\ :ExecutionProfileTest.Integration_Cassandra_TokenAwareRouting\ :ExecutionProfileTest.Integration_Cassandra_SpeculativeExecutionPolicy\ @@ -117,7 +116,6 @@ CASSANDRA_TEST_FILTER := $(subst ${SPACE},${EMPTY},ClusterTests.*\ :SchemaMetadataTest.Integration_Cassandra_RegularMetadataNotMarkedVirtual\ :SchemaMetadataTest.Integration_Cassandra_VirtualMetadata\ :HeartbeatTests.Integration_Cassandra_HeartbeatFailed\ -:TimestampTests.Integration_Cassandra_MonotonicTimestampGenerator\ :ExecutionProfileTest.Integration_Cassandra_RoundRobin\ :ExecutionProfileTest.Integration_Cassandra_TokenAwareRouting\ :ExecutionProfileTest.Integration_Cassandra_SpeculativeExecutionPolicy\ diff --git a/scylla-rust-wrapper/src/api.rs b/scylla-rust-wrapper/src/api.rs index be9387c7..4c47bf29 100644 --- a/scylla-rust-wrapper/src/api.rs +++ b/scylla-rust-wrapper/src/api.rs @@ -966,6 +966,8 @@ pub mod integration_testing { testing_retry_policy_ignoring_new, testing_statement_set_recording_history_listener, testing_statement_set_sleeping_history_listener, + testing_timestamp_gen_contains_timestamp, + testing_timestamp_gen_monotonic_new, }; /// Stubs of functions that must be implemented for the integration tests to compile, diff --git a/scylla-rust-wrapper/src/cluster.rs b/scylla-rust-wrapper/src/cluster.rs index 7e3c3dd0..a680d90e 100644 --- a/scylla-rust-wrapper/src/cluster.rs +++ b/scylla-rust-wrapper/src/cluster.rs @@ -622,6 +622,10 @@ pub unsafe extern "C" fn cass_cluster_set_timestamp_gen( CassTimestampGen::Monotonic(monotonic_timestamp_generator) => { Some(Arc::clone(monotonic_timestamp_generator) as _) } + #[cfg(cpp_integration_testing)] + CassTimestampGen::RecordingMonotonic(recording_timestamp_generator) => { + Some(Arc::clone(recording_timestamp_generator) as _) + } }; cluster.session_builder.config.timestamp_generator = rust_timestamp_gen; diff --git a/scylla-rust-wrapper/src/testing/integration.rs b/scylla-rust-wrapper/src/testing/integration.rs index 97eeb533..9377949e 100644 --- a/scylla-rust-wrapper/src/testing/integration.rs +++ b/scylla-rust-wrapper/src/testing/integration.rs @@ -10,7 +10,7 @@ use scylla::policies::retry::RetryDecision; use crate::argconv::{ ArcFFI, BoxFFI, CConst, CMut, CassBorrowedExclusivePtr, CassBorrowedSharedPtr, - CassOwnedSharedPtr, + CassOwnedExclusivePtr, CassOwnedSharedPtr, }; use crate::cluster::CassCluster; use crate::future::{CassFuture, CassResultValue}; @@ -19,7 +19,8 @@ use crate::retry_policy::CassRetryPolicy; use crate::runtime::Runtime; use crate::statements::batch::CassBatch; use crate::statements::statement::{BoundStatement, CassStatement}; -use crate::types::{cass_bool_t, cass_int32_t, cass_uint16_t, cass_uint64_t, size_t}; +use crate::timestamp_generator::{CassTimestampGen, RecordingTimestampGenerator}; +use crate::types::{cass_bool_t, cass_int32_t, cass_int64_t, cass_uint16_t, cass_uint64_t, size_t}; #[unsafe(no_mangle)] pub unsafe extern "C" fn testing_cluster_get_connect_timeout( @@ -378,6 +379,43 @@ pub unsafe extern "C" fn testing_retry_policy_ignoring_new() )))) } +/// Creates a new monotonic timestamp generator that records all generated timestamps. +/// This is useful for testing purposes, allowing us to verify which timestamps were +/// generated during query execution. +/// The recorded timestamps can later be queried using `testing_timestamp_gen_contains_timestamp`. +#[unsafe(no_mangle)] +pub unsafe extern "C" fn testing_timestamp_gen_monotonic_new() +-> CassOwnedExclusivePtr { + BoxFFI::into_ptr(Box::new(CassTimestampGen::RecordingMonotonic(Arc::new( + RecordingTimestampGenerator::new(), + )))) +} + +/// Checks whether the given timestamp was generated by the recording monotonic +/// timestamp generator. Returns `cass_true` if found, `cass_false` otherwise. +/// +/// If the provided generator is not a recording monotonic generator, returns `cass_false`. +#[unsafe(no_mangle)] +pub unsafe extern "C" fn testing_timestamp_gen_contains_timestamp( + timestamp_gen_raw: CassBorrowedExclusivePtr, + timestamp: cass_int64_t, +) -> cass_bool_t { + let Some(timestamp_gen) = BoxFFI::as_ref(timestamp_gen_raw) else { + return 0; + }; + + match timestamp_gen { + CassTimestampGen::RecordingMonotonic(recording) => { + if recording.contains(timestamp) { + 1 + } else { + 0 + } + } + _ => 0, + } +} + /// Stubs of functions that must be implemented for the integration tests /// or examples to compile, but the proper implementation is not needed for /// the tests/examples to run, and at the same time the functions are not diff --git a/scylla-rust-wrapper/src/timestamp_generator.rs b/scylla-rust-wrapper/src/timestamp_generator.rs index 623ac293..0b31ea72 100644 --- a/scylla-rust-wrapper/src/timestamp_generator.rs +++ b/scylla-rust-wrapper/src/timestamp_generator.rs @@ -2,12 +2,48 @@ use std::sync::Arc; use std::time::Duration; use scylla::policies::timestamp_generator::MonotonicTimestampGenerator; +#[cfg(cpp_integration_testing)] +use scylla::policies::timestamp_generator::TimestampGenerator; use crate::argconv::{BoxFFI, CMut, CassOwnedExclusivePtr, FFI, FromBox}; pub enum CassTimestampGen { ServerSide, Monotonic(Arc), + #[cfg(cpp_integration_testing)] + RecordingMonotonic(Arc), +} + +/// A wrapper around `MonotonicTimestampGenerator` that records all generated timestamps. +/// This is used for integration testing purposes only. +#[cfg(cpp_integration_testing)] +#[allow(unnameable_types)] +pub struct RecordingTimestampGenerator { + inner: MonotonicTimestampGenerator, + timestamps: std::sync::Mutex>, +} + +#[cfg(cpp_integration_testing)] +impl RecordingTimestampGenerator { + pub fn new() -> Self { + Self { + inner: MonotonicTimestampGenerator::new(), + timestamps: std::sync::Mutex::new(Vec::new()), + } + } + + pub fn contains(&self, timestamp: i64) -> bool { + self.timestamps.lock().unwrap().contains(×tamp) + } +} + +#[cfg(cpp_integration_testing)] +impl TimestampGenerator for RecordingTimestampGenerator { + fn next_timestamp(&self) -> i64 { + let ts = self.inner.next_timestamp(); + self.timestamps.lock().unwrap().push(ts); + ts + } } impl FFI for CassTimestampGen { diff --git a/src/testing_rust_impls.h b/src/testing_rust_impls.h index 00ef37c9..7add57cb 100644 --- a/src/testing_rust_impls.h +++ b/src/testing_rust_impls.h @@ -54,6 +54,16 @@ CASS_EXPORT void testing_statement_set_recording_history_listener(CassStatement* // The returned pointer is allocated and must be freed with `testing_free_cstring`. CASS_EXPORT char* testing_future_get_attempted_hosts(CassFuture* future); +// Creates a new monotonic timestamp generator that records all generated timestamps. +// Recorded timestamps can be queried using `testing_timestamp_gen_contains_timestamp`. +// The returned generator must be freed with `cass_timestamp_gen_free`. +CASS_EXPORT CassTimestampGen* testing_timestamp_gen_monotonic_new(); + +// Checks whether the given timestamp was generated by the recording monotonic generator. +// Returns cass_true if the timestamp was generated, cass_false otherwise. +CASS_EXPORT cass_bool_t testing_timestamp_gen_contains_timestamp(CassTimestampGen* timestamp_gen, + cass_int64_t timestamp); + /** * Creates a new ignoring retry policy. * diff --git a/src/timestamp_generator.cpp b/src/timestamp_generator.cpp deleted file mode 100644 index 40f94e82..00000000 --- a/src/timestamp_generator.cpp +++ /dev/null @@ -1,63 +0,0 @@ -/* - Copyright (c) DataStax, Inc. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -#include "timestamp_generator.hpp" - -#include "external.hpp" -#include "get_time.hpp" -#include "logger.hpp" - -using namespace datastax::internal::core; - -int64_t MonotonicTimestampGenerator::next() { - while (true) { - int64_t last = last_.load(); - int64_t next = compute_next(last); - if (last_.compare_exchange_strong(last, next)) { - return next; - } - } -} - -// This is guaranteed to return a monotonic timestamp. If clock skew is detected -// then this method will increment the last timestamp. -int64_t MonotonicTimestampGenerator::compute_next(int64_t last) { - int64_t current = get_time_since_epoch_us(); - - if (last >= current) { // There's clock skew - // If we exceed our warning threshold then warn periodically that clock - // skew has been detected. - if (warning_threshold_us_ >= 0 && last > current + warning_threshold_us_) { - // Using a monotonic clock to prevent the effects of clock skew from properly - // triggering warnings. - int64_t now = get_time_monotonic_ns() / NANOSECONDS_PER_MILLISECOND; - int64_t last_warning = last_warning_.load(); - if (now > last_warning + warning_interval_ms_ && - last_warning_.compare_exchange_strong(last_warning, now)) { - LOG_WARN("Clock skew detected. The current time (%lld) was %lld " - "microseconds behind the last generated timestamp (%lld). " - "The next generated timestamp will be artificially incremented " - "to guarantee monotonicity.", - static_cast(current), static_cast(last - current), - static_cast(last)); - } - } - - return last + 1; - } - - return current; -} diff --git a/src/timestamp_generator.hpp b/src/timestamp_generator.hpp deleted file mode 100644 index a8cf2b6f..00000000 --- a/src/timestamp_generator.hpp +++ /dev/null @@ -1,87 +0,0 @@ -/* - Copyright (c) DataStax, Inc. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -#ifndef DATASTAX_INTERNAL_TIMESTAMP_GENERATOR_HPP -#define DATASTAX_INTERNAL_TIMESTAMP_GENERATOR_HPP - -#include "atomic.hpp" -#include "constants.hpp" -#include "external.hpp" -#include "macros.hpp" -#include "ref_counted.hpp" -//#include "request.hpp" - -#include - -namespace datastax { namespace internal { namespace core { - -class TimestampGenerator : public RefCounted { -public: - typedef SharedRefPtr Ptr; - - enum Type { SERVER_SIDE, MONOTONIC }; - - TimestampGenerator(Type type) - : type_(type) {} - - virtual ~TimestampGenerator() {} - - Type type() const { return type_; } - - virtual int64_t next() = 0; - -private: - Type type_; - -private: - DISALLOW_COPY_AND_ASSIGN(TimestampGenerator); -}; - -class ServerSideTimestampGenerator : public TimestampGenerator { -public: - ServerSideTimestampGenerator() - : TimestampGenerator(SERVER_SIDE) {} - - virtual int64_t next() { return CASS_INT64_MIN; } -}; - -class MonotonicTimestampGenerator : public TimestampGenerator { -public: - MonotonicTimestampGenerator(int64_t warning_threshold_us = 1000000, - int64_t warning_interval_ms = 1000) - : TimestampGenerator(MONOTONIC) - , last_(0) - , last_warning_(0) - , warning_threshold_us_(warning_threshold_us) - , warning_interval_ms_(warning_interval_ms < 0 ? 0 : warning_interval_ms) {} - - virtual int64_t next(); - -private: - int64_t compute_next(int64_t last); - - Atomic last_; - Atomic last_warning_; - - const int64_t warning_threshold_us_; - const int64_t warning_interval_ms_; -}; - -}}} // namespace datastax::internal::core - -EXTERNAL_TYPE(datastax::internal::core::TimestampGenerator, CassTimestampGen) - -#endif diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 15030aff..1a7b516c 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -120,7 +120,6 @@ set(CPP_DRIVER_SOURCE_FILES ${CASS_SRC_DIR}/get_time-win.cpp ${CASS_SRC_DIR}/address.cpp ${CASS_SRC_DIR}/memory.cpp - ${CASS_SRC_DIR}/timestamp_generator.cpp ${CASS_SRC_DIR}/testing.cpp ${CASS_SRC_DIR}/logger.cpp ${CASS_SRC_DIR}/testing_unimplemented.cpp diff --git a/tests/src/integration/tests/test_timestamp.cpp b/tests/src/integration/tests/test_timestamp.cpp index 40f05c52..01e4f286 100644 --- a/tests/src/integration/tests/test_timestamp.cpp +++ b/tests/src/integration/tests/test_timestamp.cpp @@ -16,46 +16,11 @@ #include "integration.hpp" -#include "timestamp_generator.hpp" - -using datastax::internal::SharedRefPtr; +extern "C" { +#include "testing_rust_impls.h" +} class TimestampTests : public Integration { -private: - /** - * Monotonic timestamp generator class to mimic cass_timestamp_gen_monotonic_new() and - * cass_timestamp_gen_monotonic_new_with_settings(). This class allows for the generated timestamp - * to be retrieved. - */ - class TestMonotonicTimestampGenerator - : public datastax::internal::core::MonotonicTimestampGenerator { - public: - typedef SharedRefPtr Ptr; - TestMonotonicTimestampGenerator(int64_t warning_threshold_us = 1000000, - int64_t warning_interval_ms = 1000) - : datastax::internal::core::MonotonicTimestampGenerator(warning_threshold_us, - warning_interval_ms) {} - - bool contains(BigInteger timestamp) { - for (std::vector::iterator it = timestamps_.begin(), end = timestamps_.end(); - it != end; ++it) { - if (timestamp == *it) { - return true; - } - } - return false; - } - - virtual int64_t next() { - int64_t timestamp = datastax::internal::core::MonotonicTimestampGenerator::next(); - timestamps_.push_back(BigInteger(timestamp)); - return timestamp; - } - - private: - std::vector timestamps_; - }; - public: void SetUp() { Integration::SetUp(); @@ -84,20 +49,7 @@ class TimestampTests : public Integration { return result.first_row().column_by_name("write_time_value"); } - TimestampGenerator timestamp_generator(int64_t warning_threshold_us = 1000000, - int64_t warning_interval_ms = 1000) { - timestamp_generator_.reset( - new TestMonotonicTimestampGenerator(warning_threshold_us, warning_interval_ms)); - timestamp_generator_->inc_ref(); - return CassTimestampGen::to(timestamp_generator_.get()); - } - - bool contains_timestamp(BigInteger timestamp) { - return timestamp_generator_->contains(timestamp); - } - private: - TestMonotonicTimestampGenerator::Ptr timestamp_generator_; Prepared prepared_insert_statement_; }; @@ -216,7 +168,8 @@ CASSANDRA_INTEGRATION_TEST_F(TimestampTests, ServerSideTimestampGeneratorBatchSt CASSANDRA_INTEGRATION_TEST_F(TimestampTests, MonotonicTimestampGenerator) { CHECK_FAILURE; SKIP_IF_CASSANDRA_VERSION_LT(2.1.0); - connect(default_cluster().with_timestamp_generator(timestamp_generator())); + TimestampGenerator generator(testing_timestamp_gen_monotonic_new()); + connect(default_cluster().with_timestamp_generator(generator)); BigInteger last_timestamp; for (int i = 0; i < 100; ++i) { @@ -224,7 +177,7 @@ CASSANDRA_INTEGRATION_TEST_F(TimestampTests, MonotonicTimestampGenerator) { session_.execute(create_insert_statement(key)); BigInteger timestamp(select_timestamp(key)); - EXPECT_TRUE(contains_timestamp(timestamp)); + EXPECT_TRUE(testing_timestamp_gen_contains_timestamp(generator.get(), timestamp.value())); if (!last_timestamp.is_null()) { EXPECT_NE(last_timestamp, timestamp);