diff options
36 files changed, 99 insertions, 296 deletions
diff --git a/storage/src/tests/distributor/ownership_transfer_safe_time_point_calculator_test.cpp b/storage/src/tests/distributor/ownership_transfer_safe_time_point_calculator_test.cpp index 817e9b33226..bcb5f7199f0 100644 --- a/storage/src/tests/distributor/ownership_transfer_safe_time_point_calculator_test.cpp +++ b/storage/src/tests/distributor/ownership_transfer_safe_time_point_calculator_test.cpp @@ -3,24 +3,10 @@ #include <vespa/storage/distributor/ownership_transfer_safe_time_point_calculator.h> #include <vespa/vespalib/gtest/gtest.h> -namespace std::chrono { - -template <typename Clock, typename Duration> -std::ostream& operator<<(std::ostream& os, - std::chrono::time_point<Clock, Duration> t) -{ - os << std::chrono::duration_cast<std::chrono::milliseconds>( - t.time_since_epoch()).count() << "ms"; - return os; -} - -} - namespace storage::distributor { using CalcType = OwnershipTransferSafeTimePointCalculator; -using Clock = CalcType::Clock; -using TimePoint = CalcType::TimePoint; +using TimePoint = vespalib::system_time; using namespace std::literals::chrono_literals; diff --git a/storage/src/tests/storageframework/clock/timetest.cpp b/storage/src/tests/storageframework/clock/timetest.cpp index 17625b4bf45..20670b3b438 100644 --- a/storage/src/tests/storageframework/clock/timetest.cpp +++ b/storage/src/tests/storageframework/clock/timetest.cpp @@ -12,10 +12,10 @@ TEST(TimeTest, testBasics) MicroSecTime timeMicros2 = timeMicros; EXPECT_EQ(timeMicros2, timeMicros); - timeMicros2 += MicroSecTime(25000); + timeMicros2 = MicroSecTime(timeMicros.getTime() + 25000); EXPECT_GT(timeMicros2, timeMicros); EXPECT_LT(timeMicros, timeMicros2); - timeMicros2 -= MicroSecTime(30000); + timeMicros2 = MicroSecTime(timeMicros2.getTime() - 30000); EXPECT_LT(timeMicros2, timeMicros); EXPECT_GT(timeMicros, timeMicros2); } diff --git a/storage/src/vespa/storage/bucketdb/bucketinfo.h b/storage/src/vespa/storage/bucketdb/bucketinfo.h index cc34cfa2879..1870d4c91d4 100644 --- a/storage/src/vespa/storage/bucketdb/bucketinfo.h +++ b/storage/src/vespa/storage/bucketdb/bucketinfo.h @@ -21,6 +21,7 @@ template <typename NodeSeq> class BucketInfoBase { protected: + //TODO: Should we use a chrono timepoint to ensure we are using same clock everywhere ? uint32_t _lastGarbageCollection; NodeSeq _nodes; public: diff --git a/storage/src/vespa/storage/distributor/distributor_stripe.cpp b/storage/src/vespa/storage/distributor/distributor_stripe.cpp index 2f150cf7250..782dd9d7e12 100644 --- a/storage/src/vespa/storage/distributor/distributor_stripe.cpp +++ b/storage/src/vespa/storage/distributor/distributor_stripe.cpp @@ -848,11 +848,7 @@ DistributorStripe::enable_cluster_state_bundle(const lib::ClusterStateBundle& ne // TODO STRIPE replace legacy func enableClusterStateBundle(new_state); if (has_bucket_ownership_change) { - using TimePoint = OwnershipTransferSafeTimePointCalculator::TimePoint; - // Note: this assumes that std::chrono::system_clock and the framework - // system clock have the same epoch, which should be a reasonable - // assumption. - TimePoint now = _component.getClock().getSystemTime(); + vespalib::system_time now = _component.getClock().getSystemTime(); _externalOperationHandler.rejectFeedBeforeTimeReached(_ownershipSafeTimeCalc->safeTimePoint(now)); } _bucketDBUpdater.handle_activated_cluster_state_bundle(); // Triggers resending of queued requests diff --git a/storage/src/vespa/storage/distributor/distributor_stripe_component.cpp b/storage/src/vespa/storage/distributor/distributor_stripe_component.cpp index 2a4b63ecee2..b47e0697a91 100644 --- a/storage/src/vespa/storage/distributor/distributor_stripe_component.cpp +++ b/storage/src/vespa/storage/distributor/distributor_stripe_component.cpp @@ -109,7 +109,7 @@ UpdateBucketDatabaseProcessor::process_entry(BucketDatabase::Entry &entry) const // distributor has run GC on it, we just have to assume this and set the // timestamp to the current time to avoid duplicate work. if (entry->getLastGarbageCollectionTime() == 0) { - entry->setLastGarbageCollectionTime(_clock.getTimeInSeconds().getTime()); + entry->setLastGarbageCollectionTime(vespalib::count_s(_clock.getSystemTime().time_since_epoch())); } entry->addNodes(_changed_nodes, _ideal_nodes); if (_reset_trusted) { diff --git a/storage/src/vespa/storage/distributor/externaloperationhandler.cpp b/storage/src/vespa/storage/distributor/externaloperationhandler.cpp index 0ada9da29c1..6cb404aaa0a 100644 --- a/storage/src/vespa/storage/distributor/externaloperationhandler.cpp +++ b/storage/src/vespa/storage/distributor/externaloperationhandler.cpp @@ -4,7 +4,6 @@ #include "crypto_uuid_generator.h" #include "top_level_distributor.h" #include "distributor_bucket_space.h" -#include "distributor_bucket_space_repo.h" #include "externaloperationhandler.h" #include "operation_sequencer.h" #include <vespa/document/base/documentid.h> @@ -18,7 +17,6 @@ #include <vespa/storage/distributor/operations/external/statbucketlistoperation.h> #include <vespa/storage/distributor/operations/external/statbucketoperation.h> #include <vespa/storage/distributor/operations/external/twophaseupdateoperation.h> -#include <vespa/storage/distributor/operations/external/updateoperation.h> #include <vespa/storage/distributor/operations/external/visitoroperation.h> #include <vespa/storageapi/message/persistence.h> #include <vespa/storageapi/message/removelocation.h> @@ -127,7 +125,7 @@ ExternalOperationHandler::makeSafeTimeRejectionResult(TimePoint unsafeTime) bool ExternalOperationHandler::checkSafeTimeReached(api::StorageCommand& cmd) { - const auto now = TimePoint(std::chrono::seconds(_node_ctx.clock().getTimeInSeconds().getTime())); + vespalib::system_time now = _node_ctx.clock().getSystemTime(); if (now < _rejectFeedBeforeTimeReached) { api::StorageReply::UP reply(cmd.makeReply()); reply->setResult(makeSafeTimeRejectionResult(now)); @@ -431,7 +429,7 @@ std::shared_ptr<Operation> ExternalOperationHandler::try_generate_get_operation( bounce_with_wrong_distribution(*cmd, *snapshot.context().default_active_cluster_state()); metrics.locked()->failures.wrongdistributor.inc(); } - return std::shared_ptr<Operation>(); + return {}; } // The snapshot is aware of whether stale reads are enabled, so we don't have to check that here. const auto* space_repo = snapshot.bucket_space_repo(); diff --git a/storage/src/vespa/storage/distributor/nodeinfo.cpp b/storage/src/vespa/storage/distributor/nodeinfo.cpp index d02a1e1906e..6bb1949d606 100644 --- a/storage/src/vespa/storage/distributor/nodeinfo.cpp +++ b/storage/src/vespa/storage/distributor/nodeinfo.cpp @@ -25,7 +25,7 @@ bool NodeInfo::isBusy(uint16_t idx) const { return false; } -void NodeInfo::setBusy(uint16_t idx, framework::MonotonicDuration for_duration) { +void NodeInfo::setBusy(uint16_t idx, vespalib::duration for_duration) { getNode(idx)._busyUntilTime = _clock.getMonotonicTime() + for_duration; } diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/garbagecollectionoperation.cpp b/storage/src/vespa/storage/distributor/operations/idealstate/garbagecollectionoperation.cpp index d3e4e49c193..5599f9fb51e 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/garbagecollectionoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/idealstate/garbagecollectionoperation.cpp @@ -242,8 +242,7 @@ void GarbageCollectionOperation::on_metadata_read_phase_done(DistributorStripeMe void GarbageCollectionOperation::update_last_gc_timestamp_in_db() { BucketDatabase::Entry dbentry = _bucketSpace->getBucketDatabase().get(getBucketId()); if (dbentry.valid()) { - dbentry->setLastGarbageCollectionTime( - _manager->node_context().clock().getTimeInSeconds().getTime()); + dbentry->setLastGarbageCollectionTime(vespalib::count_s(_manager->node_context().clock().getSystemTime().time_since_epoch())); LOG(debug, "GC(%s): Tagging bucket completed at time %u", getBucket().toString().c_str(), dbentry->getLastGarbageCollectionTime()); _bucketSpace->getBucketDatabase().update(dbentry); diff --git a/storage/src/vespa/storage/distributor/ownership_transfer_safe_time_point_calculator.cpp b/storage/src/vespa/storage/distributor/ownership_transfer_safe_time_point_calculator.cpp index 818c812ab4f..d44ea464dbc 100644 --- a/storage/src/vespa/storage/distributor/ownership_transfer_safe_time_point_calculator.cpp +++ b/storage/src/vespa/storage/distributor/ownership_transfer_safe_time_point_calculator.cpp @@ -5,11 +5,11 @@ namespace storage::distributor { -OwnershipTransferSafeTimePointCalculator::TimePoint -OwnershipTransferSafeTimePointCalculator::safeTimePoint(TimePoint now) const +vespalib::system_time +OwnershipTransferSafeTimePointCalculator::safeTimePoint(vespalib::system_time now) const { if (_max_cluster_clock_skew.count() == 0) { - return TimePoint{}; + return {}; } // Rationale: distributors always generate time stamps by taking // the current second and adding a synthetic microsecond counter. @@ -26,7 +26,7 @@ OwnershipTransferSafeTimePointCalculator::safeTimePoint(TimePoint now) const // the same whole second as another distributor already has done for // any of the buckets a node now owns. auto now_sec = std::chrono::duration_cast<std::chrono::seconds>(now.time_since_epoch()); - return TimePoint(now_sec + std::chrono::seconds(1) + _max_cluster_clock_skew); + return vespalib::system_time(now_sec + std::chrono::seconds(1) + _max_cluster_clock_skew); } } diff --git a/storage/src/vespa/storage/distributor/ownership_transfer_safe_time_point_calculator.h b/storage/src/vespa/storage/distributor/ownership_transfer_safe_time_point_calculator.h index 9e1409854b5..84cae173ecc 100644 --- a/storage/src/vespa/storage/distributor/ownership_transfer_safe_time_point_calculator.h +++ b/storage/src/vespa/storage/distributor/ownership_transfer_safe_time_point_calculator.h @@ -2,7 +2,7 @@ #pragma once -#include <chrono> +#include <vespa/vespalib/util/time.h> namespace storage::distributor { @@ -28,11 +28,7 @@ namespace storage::distributor { class OwnershipTransferSafeTimePointCalculator { std::chrono::seconds _max_cluster_clock_skew; public: - using Clock = std::chrono::system_clock; - using TimePoint = std::chrono::time_point<Clock>; - - explicit OwnershipTransferSafeTimePointCalculator( - std::chrono::seconds max_cluster_clock_skew) + explicit OwnershipTransferSafeTimePointCalculator(std::chrono::seconds max_cluster_clock_skew) : _max_cluster_clock_skew(max_cluster_clock_skew) { } @@ -41,7 +37,7 @@ public: _max_cluster_clock_skew = sec; } - TimePoint safeTimePoint(TimePoint now) const; + vespalib::system_time safeTimePoint(vespalib::system_time now) const; }; } diff --git a/storage/src/vespa/storage/distributor/pending_bucket_space_db_transition.cpp b/storage/src/vespa/storage/distributor/pending_bucket_space_db_transition.cpp index f09ef468441..62de3b50b51 100644 --- a/storage/src/vespa/storage/distributor/pending_bucket_space_db_transition.cpp +++ b/storage/src/vespa/storage/distributor/pending_bucket_space_db_transition.cpp @@ -194,7 +194,7 @@ PendingBucketSpaceDbTransition::DbMerger::addToMerger(BucketDatabase::Merger& me BucketDatabase::Entry e(bucket_id, BucketInfo()); insertInfo(e, range); if (e->getLastGarbageCollectionTime() == 0) { - e->setLastGarbageCollectionTime(framework::MicroSecTime(_creation_timestamp).getSeconds().getTime()); + e->setLastGarbageCollectionTime(framework::MicroSecTime(_creation_timestamp).getSeconds()); } e.getBucketInfo().updateTrusted(); merger.insert_before_current(bucket_id, e); @@ -212,7 +212,7 @@ PendingBucketSpaceDbTransition::DbMerger::addToInserter(BucketDatabase::Trailing BucketDatabase::Entry e(bucket_id, BucketInfo()); insertInfo(e, range); if (e->getLastGarbageCollectionTime() == 0) { - e->setLastGarbageCollectionTime(framework::MicroSecTime(_creation_timestamp).getSeconds().getTime()); + e->setLastGarbageCollectionTime(framework::MicroSecTime(_creation_timestamp).getSeconds()); } e.getBucketInfo().updateTrusted(); inserter.insert_at_end(bucket_id, e); diff --git a/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.cpp b/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.cpp index f69f9e3d427..8fce8c3137a 100644 --- a/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.cpp +++ b/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.cpp @@ -92,9 +92,8 @@ StripeBucketDBUpdater::flush() } void -StripeBucketDBUpdater::print(std::ostream& out, bool verbose, const std::string& indent) const +StripeBucketDBUpdater::print(std::ostream& out, bool, const std::string&) const { - (void) verbose; (void) indent; out << "StripeBucketDBUpdater"; } @@ -461,9 +460,8 @@ StripeBucketDBUpdater::resendDelayedMessages() } void -StripeBucketDBUpdater::convertBucketInfoToBucketList( - const std::shared_ptr<api::RequestBucketInfoReply>& repl, - uint16_t targetNode, BucketListMerger::BucketList& newList) +StripeBucketDBUpdater::convertBucketInfoToBucketList(const std::shared_ptr<api::RequestBucketInfoReply>& repl, + uint16_t targetNode, BucketListMerger::BucketList& newList) { for (const auto & entry : repl->getBucketInfo()) { LOG(debug, "Received bucket information from node %u for bucket %s: %s", targetNode, @@ -492,8 +490,7 @@ StripeBucketDBUpdater::mergeBucketInfoWithDatabase( } bool -StripeBucketDBUpdater::processSingleBucketInfoReply( - const std::shared_ptr<api::RequestBucketInfoReply> & repl) +StripeBucketDBUpdater::processSingleBucketInfoReply(const std::shared_ptr<api::RequestBucketInfoReply> & repl) { auto iter = _sentMessages.find(repl->getMsgId()); @@ -520,10 +517,8 @@ StripeBucketDBUpdater::processSingleBucketInfoReply( } void -StripeBucketDBUpdater::addBucketInfoForNode( - const BucketDatabase::Entry& e, - uint16_t node, - BucketListMerger::BucketList& existing) const +StripeBucketDBUpdater::addBucketInfoForNode(const BucketDatabase::Entry& e, uint16_t node, + BucketListMerger::BucketList& existing) { const BucketCopy* copy(e->getNode(node)); if (copy) { @@ -675,7 +670,7 @@ StripeBucketDBUpdater::MergingNodeRemover::MergingNodeRemover( } void -StripeBucketDBUpdater::MergingNodeRemover::logRemove(const document::BucketId& bucketId, const char* msg) const +StripeBucketDBUpdater::MergingNodeRemover::logRemove(const document::BucketId& bucketId, const char* msg) { LOG(spam, "Removing bucket %s: %s", bucketId.toString().c_str(), msg); } @@ -801,10 +796,10 @@ namespace { class MergingGcTimeSetter : public BucketDatabase::MergingProcessor { // time_point would be preferable, but the internal DB representation is seconds since epoch - std::chrono::seconds _last_gc_at_secs_from_epoch; + vespalib::system_time _last_gc; public: - explicit MergingGcTimeSetter(std::chrono::seconds gc_time_point) noexcept - : _last_gc_at_secs_from_epoch(gc_time_point) { + explicit MergingGcTimeSetter(vespalib::system_time gc_time_point) noexcept + : _last_gc(gc_time_point) { } ~MergingGcTimeSetter() override = default; @@ -812,7 +807,7 @@ public: Result merge(BucketDatabase::Merger& merger) override { auto& entry = merger.current_entry(); // TODO widen internal GC time type...! - entry->setLastGarbageCollectionTime(static_cast<uint32_t>(_last_gc_at_secs_from_epoch.count())); + entry->setLastGarbageCollectionTime(static_cast<uint32_t>(vespalib::count_s(_last_gc.time_since_epoch()))); return Result::Update; } @@ -821,10 +816,7 @@ public: } void StripeBucketDBUpdater::reset_all_last_gc_timestamps_to_current_time() { - // Epochs are expected to be identical between clock types - // TODO remove framework clock types in favor of std::chrono - auto now_from_epoch = std::chrono::seconds(_node_ctx.clock().getTimeInSeconds().getTime()); - MergingGcTimeSetter gc_time_setter(now_from_epoch); + MergingGcTimeSetter gc_time_setter(_node_ctx.clock().getSystemTime()); auto& repo = _op_ctx.bucket_space_repo(); for (auto& bucket_space : repo) { diff --git a/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.h b/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.h index 4b25c9c6fcb..04efe91e9e7 100644 --- a/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.h +++ b/storage/src/vespa/storage/distributor/stripe_bucket_db_updater.h @@ -144,12 +144,12 @@ private: const BucketRequest& req); void mergeBucketInfoWithDatabase(const std::shared_ptr<api::RequestBucketInfoReply>& repl, const BucketRequest& req); - void convertBucketInfoToBucketList(const std::shared_ptr<api::RequestBucketInfoReply>& repl, - uint16_t targetNode, BucketListMerger::BucketList& newList); + static void convertBucketInfoToBucketList(const std::shared_ptr<api::RequestBucketInfoReply>& repl, + uint16_t targetNode, BucketListMerger::BucketList& newList); void sendRequestBucketInfo(uint16_t node, const document::Bucket& bucket, - const std::shared_ptr<MergeReplyGuard>& mergeReply); - void addBucketInfoForNode(const BucketDatabase::Entry& e, uint16_t node, - BucketListMerger::BucketList& existing) const; + const std::shared_ptr<MergeReplyGuard>& mergeReplystatic ); + static void addBucketInfoForNode(const BucketDatabase::Entry& e, uint16_t node, + BucketListMerger::BucketList& existing); void clearReadOnlyBucketRepoDatabases(); /** * Adds all buckets contained in the bucket database @@ -197,7 +197,7 @@ private: ~MergingNodeRemover() override; Result merge(BucketDatabase::Merger&) override; - void logRemove(const document::BucketId& bucketId, const char* msg) const; + static void logRemove(const document::BucketId& bucketId, const char* msg) ; bool distributorOwnsBucket(const document::BucketId&) const; const std::vector<BucketDatabase::Entry>& getNonOwnedEntries() const noexcept { diff --git a/storage/src/vespa/storage/distributor/top_level_distributor.cpp b/storage/src/vespa/storage/distributor/top_level_distributor.cpp index b0702ac7bf0..80c096135fa 100644 --- a/storage/src/vespa/storage/distributor/top_level_distributor.cpp +++ b/storage/src/vespa/storage/distributor/top_level_distributor.cpp @@ -145,9 +145,8 @@ TopLevelDistributor::onOpen() _threadPool.start(_component.getThreadPool()); start_stripe_pool(); } else { - LOG(warning, "Not starting distributor thread as it's configured to " - "run. Unless you are just running a test tool, this is a " - "fatal error."); + LOG(warning, "Not starting distributor thread as it's configured to run. Unless you are just running a " + "test tool, this is a fatal error."); } } @@ -313,8 +312,7 @@ TopLevelDistributor::storageDistributionChanged() _next_distribution = _component.getDistribution(); } else { LOG(debug, "Got distribution change, but the distribution %s was the same as before: %s", - _component.getDistribution()->toString().c_str(), - _distribution->toString().c_str()); + _component.getDistribution()->toString().c_str(), _distribution->toString().c_str()); } } @@ -457,15 +455,14 @@ TopLevelDistributor::enable_next_config_if_changed() void TopLevelDistributor::un_inhibit_maintenance_if_safe_time_passed() { - if (_maintenance_safe_time_point.time_since_epoch().count() != 0) { - using TimePoint = OwnershipTransferSafeTimePointCalculator::TimePoint; - const auto now = TimePoint(std::chrono::seconds(_component.clock().getTimeInSeconds().getTime())); + if (vespalib::count_s(_maintenance_safe_time_point.time_since_epoch()) != 0) { + const auto now = _component.clock().getSystemTime(); if (now >= _maintenance_safe_time_point) { // Thread safe. Relaxed store is fine; stripes will eventually observe new flag status. for (auto& stripe : _stripes) { stripe->inhibit_non_activation_maintenance_operations(false); } - _maintenance_safe_time_point = TimePoint{}; + _maintenance_safe_time_point = {}; LOG(debug, "Marked all stripes as no longer inhibiting non-activation maintenance operations"); } } diff --git a/storage/src/vespa/storage/distributor/top_level_distributor.h b/storage/src/vespa/storage/distributor/top_level_distributor.h index e8b3ecbf741..aa3a7b3655d 100644 --- a/storage/src/vespa/storage/distributor/top_level_distributor.h +++ b/storage/src/vespa/storage/distributor/top_level_distributor.h @@ -208,11 +208,11 @@ private: mutable std::vector<std::shared_ptr<DistributorStatus>> _fetched_status_requests; mutable std::mutex _stripe_scan_notify_mutex; std::vector<StripeScanStats> _stripe_scan_stats; // Indices are 1-1 with _stripes entries - std::chrono::steady_clock::time_point _last_host_info_send_time; - std::chrono::milliseconds _host_info_send_delay; + vespalib::steady_time _last_host_info_send_time; + vespalib::duration _host_info_send_delay; // Ideally this would use steady_clock, but for now let's use the same semantics as // feed blocking during safe time periods. - std::chrono::system_clock::time_point _maintenance_safe_time_point; + vespalib::system_time _maintenance_safe_time_point; std::chrono::seconds _maintenance_safe_time_delay; framework::ThreadWaitInfo _tickResult; MetricUpdateHook _metricUpdateHook; diff --git a/storage/src/vespa/storage/persistence/filestorage/mergestatus.h b/storage/src/vespa/storage/persistence/filestorage/mergestatus.h index 05ffd1336a2..1bba743c6e1 100644 --- a/storage/src/vespa/storage/persistence/filestorage/mergestatus.h +++ b/storage/src/vespa/storage/persistence/filestorage/mergestatus.h @@ -6,6 +6,7 @@ #include <vespa/storageapi/messageapi/storagereply.h> #include <vespa/storageapi/message/bucket.h> #include <vespa/storageframework/generic/clock/timer.h> +#include <vespa/storageframework/generic/clock/time.h> #include <vector> #include <deque> diff --git a/storage/src/vespa/storage/persistence/mergehandler.h b/storage/src/vespa/storage/persistence/mergehandler.h index b579677ac24..bcea51f50e1 100644 --- a/storage/src/vespa/storage/persistence/mergehandler.h +++ b/storage/src/vespa/storage/persistence/mergehandler.h @@ -26,6 +26,8 @@ namespace vespalib { class ISequencedTaskExecutor; } namespace document { class Document; } namespace storage { +namespace framework { struct Clock; } + namespace spi { struct PersistenceProvider; class Context; diff --git a/storage/src/vespa/storage/storageserver/bouncer.cpp b/storage/src/vespa/storage/storageserver/bouncer.cpp index 63f1cd674f8..cfe283edb9b 100644 --- a/storage/src/vespa/storage/storageserver/bouncer.cpp +++ b/storage/src/vespa/storage/storageserver/bouncer.cpp @@ -312,7 +312,7 @@ Bouncer::onDown(const std::shared_ptr<api::StorageMessage>& msg) uint64_t timestamp = extractMutationTimestampIfAny(*msg); if (timestamp != 0) { timestamp /= 1000000; - uint64_t currentTime = _component.getClock().getTimeInSeconds().getTime(); + uint64_t currentTime = vespalib::count_s(_component.getClock().getSystemTime().time_since_epoch()); if (timestamp > currentTime + maxClockSkewInSeconds) { rejectCommandWithTooHighClockSkew(*msg, maxClockSkewInSeconds); return true; diff --git a/storage/src/vespa/storage/storageserver/distributornode.cpp b/storage/src/vespa/storage/storageserver/distributornode.cpp index 5f39c7abada..45abd34e131 100644 --- a/storage/src/vespa/storage/storageserver/distributornode.cpp +++ b/storage/src/vespa/storage/storageserver/distributornode.cpp @@ -111,7 +111,7 @@ DistributorNode::createChain(IStorageChainBuilder &builder) api::Timestamp DistributorNode::generate_unique_timestamp() { - uint64_t now_seconds = _component->getClock().getTimeInSeconds().getTime(); + uint64_t now_seconds = vespalib::count_s(_component->getClock().getSystemTime().time_since_epoch()); std::lock_guard lock(_timestamp_mutex); // We explicitly handle a seemingly decreased wall clock time, as multiple threads may // race with each other over a second change edge. In this case, pretend an earlier diff --git a/storage/src/vespa/storage/storageserver/distributornodecontext.cpp b/storage/src/vespa/storage/storageserver/distributornodecontext.cpp index c74490a18aa..f3aca7a427d 100644 --- a/storage/src/vespa/storage/storageserver/distributornodecontext.cpp +++ b/storage/src/vespa/storage/storageserver/distributornodecontext.cpp @@ -1,13 +1,12 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "distributornodecontext.h" +#include <vespa/storageframework/generic/clock/clock.h> namespace storage { -DistributorNodeContext::DistributorNodeContext( - framework::Clock::UP clock) - : StorageNodeContext(StorageComponentRegisterImpl::UP(new DistributorComponentRegisterImpl), - std::move(clock)), +DistributorNodeContext::DistributorNodeContext(std::unique_ptr<framework::Clock> clock) + : StorageNodeContext(std::make_unique<DistributorComponentRegisterImpl>(), std::move(clock)), _componentRegister(dynamic_cast<ComponentRegister&>(StorageNodeContext::getComponentRegister())) { } diff --git a/storage/src/vespa/storage/storageserver/distributornodecontext.h b/storage/src/vespa/storage/storageserver/distributornodecontext.h index d01127827e0..5691d014d1f 100644 --- a/storage/src/vespa/storage/storageserver/distributornodecontext.h +++ b/storage/src/vespa/storage/storageserver/distributornodecontext.h @@ -25,8 +25,7 @@ struct DistributorNodeContext : public StorageNodeContext { * You can provide your own clock implementation. Useful in testing where * you want to fake the clock. */ - DistributorNodeContext( - framework::Clock::UP clock = framework::Clock::UP(new RealClock)); + DistributorNodeContext(std::unique_ptr<framework::Clock> clock); /** * Get the actual component register. Available as the actual type as the diff --git a/storage/src/vespa/storage/storageserver/servicelayernodecontext.cpp b/storage/src/vespa/storage/storageserver/servicelayernodecontext.cpp index 544477c7ca5..12985d2476f 100644 --- a/storage/src/vespa/storage/storageserver/servicelayernodecontext.cpp +++ b/storage/src/vespa/storage/storageserver/servicelayernodecontext.cpp @@ -1,10 +1,11 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "servicelayernodecontext.h" +#include <vespa/storageframework/generic/clock/clock.h> namespace storage { -ServiceLayerNodeContext::ServiceLayerNodeContext(framework::Clock::UP clock, const ContentBucketDbOptions& db_opts) +ServiceLayerNodeContext::ServiceLayerNodeContext(std::unique_ptr<framework::Clock> clock, const ContentBucketDbOptions& db_opts) : StorageNodeContext(std::make_unique<ServiceLayerComponentRegisterImpl>(db_opts), std::move(clock)), _componentRegister(dynamic_cast<ComponentRegister&>(StorageNodeContext::getComponentRegister())) diff --git a/storage/src/vespa/storage/storageserver/servicelayernodecontext.h b/storage/src/vespa/storage/storageserver/servicelayernodecontext.h index d89ba472f83..72cf95ef120 100644 --- a/storage/src/vespa/storage/storageserver/servicelayernodecontext.h +++ b/storage/src/vespa/storage/storageserver/servicelayernodecontext.h @@ -25,7 +25,7 @@ struct ServiceLayerNodeContext : public StorageNodeContext { * You can provide your own clock implementation. Useful in testing where * you want to fake the clock. */ - ServiceLayerNodeContext(framework::Clock::UP clock, const ContentBucketDbOptions& db_opts); + ServiceLayerNodeContext(std::unique_ptr<framework::Clock> clock, const ContentBucketDbOptions& db_opts); /** * Get the actual component register. Available as the actual type as the diff --git a/storage/src/vespa/storage/storageserver/storagenodecontext.cpp b/storage/src/vespa/storage/storageserver/storagenodecontext.cpp index df087208425..ae7948a2916 100644 --- a/storage/src/vespa/storage/storageserver/storagenodecontext.cpp +++ b/storage/src/vespa/storage/storageserver/storagenodecontext.cpp @@ -1,10 +1,11 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "storagenodecontext.h" +#include <vespa/storageframework/generic/clock/clock.h> namespace storage { -StorageNodeContext::StorageNodeContext(ComponentRegister::UP compReg, framework::Clock::UP clock) +StorageNodeContext::StorageNodeContext(std::unique_ptr<ComponentRegister> compReg, std::unique_ptr<framework::Clock> clock) : _componentRegister(std::move(compReg)), _clock(std::move(clock)), _threadPool(*_clock) diff --git a/storage/src/vespa/storage/storageserver/storagenodecontext.h b/storage/src/vespa/storage/storageserver/storagenodecontext.h index c89bb4630ce..f07bdd37cd4 100644 --- a/storage/src/vespa/storage/storageserver/storagenodecontext.h +++ b/storage/src/vespa/storage/storageserver/storagenodecontext.h @@ -13,7 +13,6 @@ #pragma once #include <vespa/storage/frameworkimpl/component/storagecomponentregisterimpl.h> -#include <vespa/storageframework/defaultimplementation/clock/realclock.h> #include <vespa/storageframework/defaultimplementation/thread/threadpoolimpl.h> namespace storage { @@ -21,7 +20,6 @@ namespace storage { struct StorageNodeContext { // Typedefs to simplify the remainder of the interface using ComponentRegister = StorageComponentRegisterImpl; - using RealClock = framework::defaultimplementation::RealClock; /** * Get the actual component register. Available as the actual type as the @@ -41,11 +39,11 @@ struct StorageNodeContext { protected: // Initialization has been split in two as subclass needs to initialize // component register before sending it on. - StorageNodeContext(ComponentRegister::UP, framework::Clock::UP); + StorageNodeContext(std::unique_ptr<ComponentRegister>, std::unique_ptr<framework::Clock>); private: - ComponentRegister::UP _componentRegister; - framework::Clock::UP _clock; + std::unique_ptr<ComponentRegister> _componentRegister; + std::unique_ptr<framework::Clock> _clock; framework::defaultimplementation::ThreadPoolImpl _threadPool; }; diff --git a/storage/src/vespa/storage/visiting/commandqueue.h b/storage/src/vespa/storage/visiting/commandqueue.h index c9c34b7b4e0..c6cfd60c628 100644 --- a/storage/src/vespa/storage/visiting/commandqueue.h +++ b/storage/src/vespa/storage/visiting/commandqueue.h @@ -87,8 +87,6 @@ public: _cached_size(0) {} - const framework::Clock& getTimer() const { return _clock; } - iterator begin() { return _commands.begin(); } iterator end() { return _commands.end(); } diff --git a/storage/src/vespa/storageframework/defaultimplementation/clock/fakeclock.h b/storage/src/vespa/storageframework/defaultimplementation/clock/fakeclock.h index ed41014c443..395544fd175 100644 --- a/storage/src/vespa/storageframework/defaultimplementation/clock/fakeclock.h +++ b/storage/src/vespa/storageframework/defaultimplementation/clock/fakeclock.h @@ -60,9 +60,7 @@ public: } int64_t getTimeInMicros() const; - framework::SecondTime getTimeInSeconds() const override { - return SecondTime(vespalib::count_s(getSystemTime().time_since_epoch())); - } + vespalib::system_time getSystemTime() const override { // For simplicity, assume fake monotonic time follows fake wall clock. return vespalib::system_time(std::chrono::microseconds(getTimeInMicros())); diff --git a/storage/src/vespa/storageframework/defaultimplementation/clock/realclock.cpp b/storage/src/vespa/storageframework/defaultimplementation/clock/realclock.cpp index d26e6a43ed0..9beb55bdef3 100644 --- a/storage/src/vespa/storageframework/defaultimplementation/clock/realclock.cpp +++ b/storage/src/vespa/storageframework/defaultimplementation/clock/realclock.cpp @@ -5,13 +5,6 @@ namespace storage::framework::defaultimplementation { -SecondTime -RealClock::getTimeInSeconds() const { - struct timeval mytime; - gettimeofday(&mytime, nullptr); - return SecondTime(mytime.tv_sec); -} - vespalib::steady_time RealClock::getMonotonicTime() const { return vespalib::steady_clock::now(); diff --git a/storage/src/vespa/storageframework/defaultimplementation/clock/realclock.h b/storage/src/vespa/storageframework/defaultimplementation/clock/realclock.h index a1f455f13a6..3a9a127defe 100644 --- a/storage/src/vespa/storageframework/defaultimplementation/clock/realclock.h +++ b/storage/src/vespa/storageframework/defaultimplementation/clock/realclock.h @@ -14,7 +14,6 @@ namespace storage::framework::defaultimplementation { struct RealClock : public Clock { - SecondTime getTimeInSeconds() const override; vespalib::steady_time getMonotonicTime() const override; vespalib::system_time getSystemTime() const override; }; diff --git a/storage/src/vespa/storageframework/generic/clock/clock.h b/storage/src/vespa/storageframework/generic/clock/clock.h index 2e431ba8a17..5a591fa0718 100644 --- a/storage/src/vespa/storageframework/generic/clock/clock.h +++ b/storage/src/vespa/storageframework/generic/clock/clock.h @@ -13,18 +13,13 @@ #pragma once -#include "time.h" -#include <memory> +#include <vespa/vespalib/util/time.h> namespace storage::framework { struct Clock { - using UP = std::unique_ptr<Clock>; - virtual ~Clock() = default; - [[nodiscard]] virtual SecondTime getTimeInSeconds() const = 0; - // Time point resolution is intentionally not defined here. [[nodiscard]] virtual vespalib::steady_time getMonotonicTime() const = 0; [[nodiscard]] virtual vespalib::system_time getSystemTime() const = 0; diff --git a/storage/src/vespa/storageframework/generic/clock/time.cpp b/storage/src/vespa/storageframework/generic/clock/time.cpp index 64434779c91..a27caa44d40 100644 --- a/storage/src/vespa/storageframework/generic/clock/time.cpp +++ b/storage/src/vespa/storageframework/generic/clock/time.cpp @@ -1,83 +1,17 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include "time.hpp" -#include "clock.h" -#include <iomanip> -#include <vector> -#include <cassert> -#include <sstream> +#include "time.h" +#include <vespa/vespalib/stllike/asciistream.h> namespace storage::framework { -namespace { - void detectUnit(uint64_t& val, const char* unit, uint64_t size, - std::vector<std::pair<uint64_t, vespalib::string> >& units) { - if (val / size > 0) { - uint64_t value = val / size; - vespalib::string unitname = unit; - if (value != 1) unitname += "s"; - units.emplace_back(value, unitname); - val -= value * size; - } - } +std::ostream& operator<<(std::ostream& out, const MicroSecTime & t) { + return out << t.getTime(); } -vespalib::string -getTimeString(uint64_t microSecondTime, TimeFormat format) -{ - // Rewrite to use other type of stream later if needed for performance - std::ostringstream ost; - if (format & DIFFERENCE_ALL) { - std::vector<std::pair<uint64_t, vespalib::string> > vals; - detectUnit(microSecondTime, "day", 24 * 60 * 60 * 1000 * 1000ull, vals); - detectUnit(microSecondTime, "hour", 60 * 60 * 1000 * 1000ull, vals); - detectUnit(microSecondTime, "minute", 60 * 1000 * 1000, vals); - detectUnit(microSecondTime, "second", 1000 * 1000, vals); - if (format & DIFFERENCE_WITH_MICROS) { - detectUnit(microSecondTime, "microsecond", 1, vals); - if (vals.empty()) { ost << "0 microseconds"; } - } else { - if (vals.empty()) { ost << "0 seconds"; } - } - if (vals.empty()) { - return ost.str(); - } - ost << vals[0].first << " " << vals[0].second; - for (uint32_t i=1; i<vals.size(); ++i) { - if (i + 1 >= vals.size()) { - ost << " and "; - } else { - ost << ", "; - } - ost << vals[i].first << " " << vals[i].second; - } - return ost.str(); - } - time_t secondTime = microSecondTime / 1000000; - struct tm datestruct; - struct tm* datestructptr = gmtime_r(&secondTime, &datestruct); - assert(datestructptr); - (void) datestructptr; - ost << std::setfill('0') - << std::setw(4) << (datestruct.tm_year + 1900) - << '-' << std::setw(2) << (datestruct.tm_mon + 1) - << '-' << std::setw(2) << datestruct.tm_mday - << ' ' << std::setw(2) << datestruct.tm_hour - << ':' << std::setw(2) << datestruct.tm_min - << ':' << std::setw(2) << datestruct.tm_sec; - uint64_t micros = microSecondTime % 1000000; - if (format == DATETIME_WITH_MILLIS) { - ost << '.' << std::setw(3) << (micros / 1000); - } else if (format == DATETIME_WITH_MICROS) { - ost << '.' << std::setw(6) << micros; - } - return ost.str(); -} - -template std::ostream& operator<< <MicroSecTime, 1>(std::ostream&, const Time<MicroSecTime, 1> &); -template std::ostream& operator<< <SecondTime, 1000000>(std::ostream&, const Time<SecondTime, 1000000> &); -template vespalib::asciistream& operator<< <MicroSecTime, 1>(vespalib::asciistream &, const Time<MicroSecTime, 1> &); -template vespalib::asciistream& operator<< <SecondTime, 1000000>(vespalib::asciistream &, const Time<SecondTime, 1000000> &); +vespalib::asciistream& operator<<(vespalib::asciistream& out, const MicroSecTime & t) { + return out << t.getTime(); +} } diff --git a/storage/src/vespa/storageframework/generic/clock/time.h b/storage/src/vespa/storageframework/generic/clock/time.h index 8e2a654b3ed..7d2b3602432 100644 --- a/storage/src/vespa/storageframework/generic/clock/time.h +++ b/storage/src/vespa/storageframework/generic/clock/time.h @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once -#include <vespa/vespalib/stllike/string.h> #include <vespa/vespalib/util/time.h> namespace vespalib { @@ -10,85 +9,6 @@ namespace vespalib { namespace storage::framework { -using MonotonicTimePoint = vespalib::steady_time; -using MonotonicDuration = vespalib::duration; - -struct Clock; - -enum TimeFormat { - DATETIME = 0x01, // 2010-04-26 19:23:03 - DATETIME_WITH_MILLIS = 0x02, // 2010-04-26 19:23:03.001 - DATETIME_WITH_MICROS = 0x04, // 2010-04-26 19:23:03.001023 - DATETIME_ALL = 0x07, - DIFFERENCE = 0x10, // 1 day, 4 hours, 43 minutes and 3 seconds - DIFFERENCE_WITH_MICROS = 0x20, // 1 day, 4 hours, 43 minutes, 3 seconds and 123123 microseconds - DIFFERENCE_ALL = 0x30 -}; - -/** - * Utility function used by Time instances (to avoid implementation in - * header file). - */ -vespalib::string getTimeString(uint64_t microSecondTime, TimeFormat format); - -// TODO deprecate framework time point and duration classes in favor of -// using std::chrono. - -/** - * Class containing common functionality for the various time instances. Try to - * make time instances as easy to use as possible, without creating risk of - * automatic conversion between time types. - */ -template<typename Type, int MicrosPerUnit> -class Time -{ - uint64_t _time; // time_t may be signed. Negative timestamps is just a - // source for bugs. Enforce unsigned. - -protected: - explicit Time(uint64_t t) : _time(t) {} - -public: - [[nodiscard]] uint64_t getTime() const { return _time; } - - Type& operator-=(const Type& o) { _time -= o._time; return static_cast<Type&>(*this); } - Type& operator+=(const Type& o) { _time += o._time; return static_cast<Type&>(*this); } - bool operator<(const Type& o) const { return (_time < o._time); } - bool operator<=(const Type& o) const { return (_time <= o._time); } - bool operator>=(const Type& o) const { return (_time >= o._time); } - bool operator>(const Type& o) const { return (_time > o._time); } - bool operator==(const Type& o) const { return (_time == o._time); } - Type& operator++() { ++_time; return static_cast<Type&>(*this); } - Type& operator--() { --_time; return *this; } - - [[nodiscard]] vespalib::string toString(TimeFormat timeFormat = DATETIME) const { - return getTimeString(_time * MicrosPerUnit, timeFormat); - } - - static Type max() { return Type(std::numeric_limits<uint64_t>::max()); } - -}; - -template<typename Type, int MPU> -std::ostream& operator<<(std::ostream& out, const Time<Type, MPU>& t); - -template<typename Type, int MPU> -vespalib::asciistream& operator<<(vespalib::asciistream& out, const Time<Type, MPU>& t); - -/** - * \class storage::framework::SecondTime - * \ingroup clock - * - * \brief Wrapper class for a timestamp in seconds. - * - * To prevent errors where one passes time in one granularity to a function - * requiring time in another granularity. This little wrapper class exist to - * make sure that will conflict in types - */ -struct SecondTime : public Time<SecondTime, 1000000> { - explicit SecondTime(uint64_t t = 0) : Time<SecondTime, 1000000>(t) {} -}; - /** * \class storage::framework::MicroSecTime * \ingroup clock @@ -99,10 +19,29 @@ struct SecondTime : public Time<SecondTime, 1000000> { * requiring time in another granularity. This little wrapper class exist to * make sure that will conflict in types */ -struct MicroSecTime : public Time<MicroSecTime, 1> { - explicit MicroSecTime(uint64_t t = 0) : Time<MicroSecTime, 1>(t) {} +class MicroSecTime +{ +public: + explicit MicroSecTime(uint64_t t) noexcept : _time(t) {} - [[nodiscard]] SecondTime getSeconds() const { return SecondTime(getTime() / 1000000); } + [[nodiscard]] uint64_t getTime() const noexcept { return _time; } + + bool operator<(const MicroSecTime& o) const noexcept { return (_time < o._time); } + bool operator<=(const MicroSecTime& o) const noexcept { return (_time <= o._time); } + bool operator>=(const MicroSecTime& o) const noexcept { return (_time >= o._time); } + bool operator>(const MicroSecTime& o) const noexcept { return (_time > o._time); } + bool operator==(const MicroSecTime& o) const noexcept { return (_time == o._time); } + + [[nodiscard]] uint32_t getSeconds() const noexcept { return (getTime() / 1000000); } + + static MicroSecTime max() noexcept { return MicroSecTime(std::numeric_limits<uint64_t>::max()); } +private: + // time_t may be signed. Negative timestamps is just a source for bugs. Enforce unsigned. + uint64_t _time; }; +std::ostream& operator<<(std::ostream& out, const MicroSecTime & t); + +vespalib::asciistream& operator<<(vespalib::asciistream& out, const MicroSecTime & t); + } diff --git a/storage/src/vespa/storageframework/generic/clock/time.hpp b/storage/src/vespa/storageframework/generic/clock/time.hpp deleted file mode 100644 index 2cc3fb7be9d..00000000000 --- a/storage/src/vespa/storageframework/generic/clock/time.hpp +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#pragma once - -#include "time.h" -#include <vespa/vespalib/stllike/asciistream.h> - -namespace storage { -namespace framework { - -template<typename Type, int MPU> -std::ostream& operator<<(std::ostream& out, const Time<Type, MPU>& t) { - return out << t.getTime(); -} - -template<typename Type, int MPU> -vespalib::asciistream& operator<<(vespalib::asciistream& out, const Time<Type, MPU>& t) { - return out << t.getTime(); -} - -} // framework -} // storage diff --git a/storage/src/vespa/storageframework/generic/clock/timer.h b/storage/src/vespa/storageframework/generic/clock/timer.h index 82b2fcc448e..aabef4601e0 100644 --- a/storage/src/vespa/storageframework/generic/clock/timer.h +++ b/storage/src/vespa/storageframework/generic/clock/timer.h @@ -14,7 +14,7 @@ namespace storage::framework { class MilliSecTimer { const Clock* _clock; - MonotonicTimePoint _startTime; + vespalib::steady_time _startTime; public: explicit MilliSecTimer(const Clock& clock) @@ -26,7 +26,7 @@ public: MilliSecTimer(const MilliSecTimer&) = default; MilliSecTimer& operator=(const MilliSecTimer&) = default; - [[nodiscard]] MonotonicDuration getElapsedTime() const { + [[nodiscard]] vespalib::duration getElapsedTime() const { return _clock->getMonotonicTime() - _startTime; } diff --git a/storageserver/src/vespa/storageserver/app/distributorprocess.cpp b/storageserver/src/vespa/storageserver/app/distributorprocess.cpp index 7e65ff68601..987b3b613d8 100644 --- a/storageserver/src/vespa/storageserver/app/distributorprocess.cpp +++ b/storageserver/src/vespa/storageserver/app/distributorprocess.cpp @@ -5,6 +5,7 @@ #include <vespa/storage/common/bucket_stripe_utils.h> #include <vespa/storage/common/i_storage_chain_builder.h> #include <vespa/storage/common/storagelink.h> +#include <vespa/storageframework/defaultimplementation/clock/realclock.h> #include <thread> #include <vespa/log/log.h> @@ -14,7 +15,7 @@ namespace storage { DistributorProcess::DistributorProcess(const config::ConfigUri & configUri) : Process(configUri), - _context(), + _context(std::make_unique<framework::defaultimplementation::RealClock>()), _num_distributor_stripes(0), // TODO STRIPE: change default when legacy single stripe mode is removed _node(), _distributorConfigHandler(), diff --git a/storageserver/src/vespa/storageserver/app/servicelayerprocess.cpp b/storageserver/src/vespa/storageserver/app/servicelayerprocess.cpp index e6af40a552d..ab962fc78b2 100644 --- a/storageserver/src/vespa/storageserver/app/servicelayerprocess.cpp +++ b/storageserver/src/vespa/storageserver/app/servicelayerprocess.cpp @@ -6,6 +6,7 @@ #include <vespa/storage/common/i_storage_chain_builder.h> #include <vespa/storage/config/config-stor-server.h> #include <vespa/storage/storageserver/servicelayernode.h> +#include <vespa/storageframework/defaultimplementation/clock/realclock.h> #include <vespa/searchvisitor/searchvisitor.h> #include <vespa/log/log.h> |