From 840aacbbae61a4d0162d2decb534a6fe5fae030a Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Fri, 9 Aug 2019 13:34:18 +0000 Subject: Remove visitor ordering and order selection. --- .../operations/external/visitoroperation.cpp | 165 +++------------------ .../operations/external/visitoroperation.h | 6 - .../distributor/operations/external/visitororder.h | 6 +- .../storage/storageserver/documentapiconverter.cpp | 2 - storage/src/vespa/storage/visiting/visitor.cpp | 61 +------- storage/src/vespa/storage/visiting/visitor.h | 5 +- .../src/vespa/storage/visiting/visitorthread.cpp | 8 - 7 files changed, 25 insertions(+), 228 deletions(-) (limited to 'storage') diff --git a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp index a89dd52775b..f777f5de7c3 100644 --- a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp @@ -8,7 +8,6 @@ #include #include #include -#include #include #include #include @@ -344,78 +343,6 @@ VisitorOperation::verifyCreateVisitorCommand(DistributorMessageSender& sender) } } -namespace { - -bool -isSplitPastOrderBits(const document::BucketId& bucket, - const document::OrderingSpecification& ordering) { - int32_t bitsUsed = bucket.getUsedBits(); - int32_t orderBitCount = ordering.getWidthBits() - - ordering.getDivisionBits(); - return (bitsUsed > 32 + orderBitCount); -} - -bool -isInconsistentlySplit(const document::BucketId& ain, - const document::BucketId& bin) { - int minUsed = std::min(ain.getUsedBits(), bin.getUsedBits()); - - document::BucketId a = document::BucketId(minUsed, - ain.getRawId()).stripUnused(); - document::BucketId b = document::BucketId(minUsed, - bin.getRawId()).stripUnused(); - - return (a == b); -} - -bool -isInconsistentlySplit(const document::BucketId& bucket, - const std::vector& buckets) -{ - if (buckets.size()) { - for (uint32_t i=0; i::const_iterator -VisitorOperation::addSpecialBucketsForOrderDoc( - std::vector::const_iterator iter, - std::vector::const_iterator end) -{ - if (_ordering->getWidthBits() == 0) { - return iter; - } - for (; iter != end; ++iter) { - if (isSpecialBucketForOrderDoc(*iter)) { - _superBucket.subBucketsVisitOrder.push_back(*iter); - _superBucket.subBuckets[*iter] = BucketInfo(); - } else { - break; - } - } - return iter; -} - bool VisitorOperation::pickBucketsToVisit(const std::vector& buckets) { @@ -427,7 +354,7 @@ VisitorOperation::pickBucketsToVisit(const std::vector& b bucketVisitOrder.push_back(buckets[i].getBucketId()); } - VisitorOrder bucketLessThan(*_ordering); + VisitorOrder bucketLessThan; std::sort(bucketVisitOrder.begin(), bucketVisitOrder.end(), bucketLessThan); std::vector::const_iterator iter(bucketVisitOrder.begin()); @@ -451,8 +378,6 @@ VisitorOperation::pickBucketsToVisit(const std::vector& b } } - iter = addSpecialBucketsForOrderDoc(iter, end); - bool doneExpand(iter == bucketVisitOrder.end()); return doneExpand; } @@ -539,9 +464,7 @@ VisitorOperation::expandBucketContained() _superBucket.subBucketsVisitOrder.push_back(*bid); _superBucket.subBuckets[*bid] = BucketInfo(); - bid = getBucketIdAndLast(_bucketSpace.getBucketDatabase(), - _superBucket.bid, - *bid); + bid = getBucketIdAndLast(_bucketSpace.getBucketDatabase(), _superBucket.bid, *bid); } bool doneExpand = (!bid.get() || !_superBucket.bid.contains(*bid)); @@ -552,25 +475,21 @@ void VisitorOperation::expandBucket() { bool doneExpandBuckets = false; - if (_ordering->getWidthBits() > 0) { // Orderdoc - doneExpandBuckets = expandBucketAll(); + bool doneExpandContainingBuckets = true; + if (!_superBucket.bid.contains(_lastBucket)) { + LOG(spam, "Bucket %s does not contain progress bucket %s", + _superBucket.bid.toString().c_str(), + _lastBucket.toString().c_str()); + doneExpandContainingBuckets = expandBucketContaining(); } else { - bool doneExpandContainingBuckets = true; - if (!_superBucket.bid.contains(_lastBucket)) { - LOG(spam, "Bucket %s does not contain progress bucket %s", - _superBucket.bid.toString().c_str(), - _lastBucket.toString().c_str()); - doneExpandContainingBuckets = expandBucketContaining(); - } else { - LOG(spam, "Bucket %s contains progress bucket %s", - _superBucket.bid.toString().c_str(), - _lastBucket.toString().c_str()); - } + LOG(spam, "Bucket %s contains progress bucket %s", + _superBucket.bid.toString().c_str(), + _lastBucket.toString().c_str()); + } - if (doneExpandContainingBuckets) { - LOG(spam, "Done expanding containing buckets"); - doneExpandBuckets = expandBucketContained(); - } + if (doneExpandContainingBuckets) { + LOG(spam, "Done expanding containing buckets"); + doneExpandBuckets = expandBucketContained(); } if (doneExpandBuckets) { @@ -588,8 +507,7 @@ VisitorOperation::expandBucket() namespace { bool -alreadyTried(const std::vector& triedNodes, - uint16_t node) +alreadyTried(const std::vector& triedNodes, uint16_t node) { for (uint32_t j = 0; j < triedNodes.size(); j++) { if (triedNodes[j] == node) { @@ -649,57 +567,8 @@ VisitorOperation::pickTargetNode( } bool -VisitorOperation::documentSelectionMayHaveOrdering() const +VisitorOperation::parseDocumentSelection(DistributorMessageSender& ) { - // FIXME: this is hairy and depends on opportunistic ordering - // parsing working fine even when no ordering is present. - return strcasestr(_msg->getDocumentSelection().c_str(), "order") != NULL; -} - -void -VisitorOperation::attemptToParseOrderingSelector() -{ - std::unique_ptr docSelection; - std::shared_ptr repo(_owner.getTypeRepo()); - document::select::Parser parser( - *repo, _owner.getBucketIdFactory()); - docSelection = parser.parse(_msg->getDocumentSelection()); - - document::OrderingSelector selector; - _ordering = selector.select(*docSelection, _msg->getVisitorOrdering()); -} - -bool -VisitorOperation::parseDocumentSelection(DistributorMessageSender& sender) -{ - try{ - if (documentSelectionMayHaveOrdering()) { - attemptToParseOrderingSelector(); - } - - if (!_ordering.get()) { - _ordering.reset(new document::OrderingSpecification()); - } - } catch (document::DocumentTypeNotFoundException& e) { - std::ostringstream ost; - ost << "Failed to parse document select string '" - << _msg->getDocumentSelection() << "': " << e.getMessage(); - LOG(warning, "CreateVisitor(%s): %s", - _msg->getInstanceId().c_str(), ost.str().c_str()); - - sendReply(api::ReturnCode(api::ReturnCode::ILLEGAL_PARAMETERS, ost.str()), sender); - return false; - } catch (document::select::ParsingFailedException& e) { - std::ostringstream ost; - ost << "Failed to parse document select string '" - << _msg->getDocumentSelection() << "': " << e.getMessage(); - LOG(warning, "CreateVisitor(%s): %s", - _msg->getInstanceId().c_str(), ost.str().c_str()); - - sendReply(api::ReturnCode(api::ReturnCode::ILLEGAL_PARAMETERS, ost.str()), sender); - return false; - } - return true; } diff --git a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h index bc0308d8ef5..abf00c51f12 100644 --- a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h @@ -134,10 +134,6 @@ private: * code value, which avoids overwriting more critical errors. */ void markOperationAsFailed(const api::ReturnCode& result); - bool isSpecialBucketForOrderDoc(const document::BucketId& bucketId) const; - std::vector::const_iterator addSpecialBucketsForOrderDoc( - std::vector::const_iterator iter, - std::vector::const_iterator end); /** * Compute time remaining of visitor in milliseconds, relative to timeout * time point. In case of the current time having passed the timeout @@ -145,8 +141,6 @@ private: */ uint64_t timeLeft() const noexcept; - std::unique_ptr _ordering; - DistributorComponent& _owner; DistributorBucketSpace &_bucketSpace; SentMessagesMap _sentMessages; diff --git a/storage/src/vespa/storage/distributor/operations/external/visitororder.h b/storage/src/vespa/storage/distributor/operations/external/visitororder.h index 69aefb1ec33..89a5d9e3734 100644 --- a/storage/src/vespa/storage/distributor/operations/external/visitororder.h +++ b/storage/src/vespa/storage/distributor/operations/external/visitororder.h @@ -5,10 +5,10 @@ namespace storage::distributor { struct VisitorOrder { - const document::OrderingSpecification& _ordering; + document::OrderingSpecification _ordering; - VisitorOrder(const document::OrderingSpecification& ordering) - : _ordering(ordering) {} + VisitorOrder() + : _ordering() {} document::BucketId::Type getOrder(const document::BucketId& bid) { int32_t orderBitCount = _ordering.getWidthBits() - diff --git a/storage/src/vespa/storage/storageserver/documentapiconverter.cpp b/storage/src/vespa/storage/storageserver/documentapiconverter.cpp index f9deb76c3e2..54b1a9ae257 100644 --- a/storage/src/vespa/storage/storageserver/documentapiconverter.cpp +++ b/storage/src/vespa/storage/storageserver/documentapiconverter.cpp @@ -89,7 +89,6 @@ DocumentApiConverter::toStorageAPI(documentapi::DocumentMessage& fromMsg) to->setVisitInconsistentBuckets(from.visitInconsistentBuckets()); to->getBuckets() = from.getBuckets(); to->setVisitorDispatcherVersion(from.getVisitorDispatcherVersion()); - to->setVisitorOrdering(from.getVisitorOrdering()); to->setMaxBucketsPerVisitor(from.getMaxBucketsPerVisitor()); toMsg = std::move(to); break; @@ -286,7 +285,6 @@ DocumentApiConverter::toDocumentAPI(api::StorageCommand& fromMsg) to->setFieldSet(from.getFieldSet()); to->setVisitInconsistentBuckets(from.visitInconsistentBuckets()); to->getBuckets() = from.getBuckets(); - to->setVisitorOrdering(from.getVisitorOrdering()); to->setMaxBucketsPerVisitor(from.getMaxBucketsPerVisitor()); toMsg = std::move(to); break; diff --git a/storage/src/vespa/storage/visiting/visitor.cpp b/storage/src/vespa/storage/visiting/visitor.cpp index 97ae59d3344..3bed02f88fe 100644 --- a/storage/src/vespa/storage/visiting/visitor.cpp +++ b/storage/src/vespa/storage/visiting/visitor.cpp @@ -20,70 +20,19 @@ using document::BucketSpace; namespace storage { -Visitor::HitCounter::HitCounter(const document::OrderingSpecification* ordering) +Visitor::HitCounter::HitCounter() : _firstPassHits(0), _firstPassBytes(0), _secondPassHits(0), - _secondPassBytes(0), - _ordering(ordering) + _secondPassBytes(0) { } void -Visitor::HitCounter::addHit(const document::DocumentId& hit, uint32_t size) +Visitor::HitCounter::addHit(const document::DocumentId& , uint32_t size) { bool firstPass = false; - if (_ordering && _ordering->getWidthBits() > 0 - && hit.getScheme().getType() == document::IdString::ORDERDOC) - { - const document::OrderDocIdString& order( - static_cast(hit.getScheme())); - - int32_t width = (1 << order.getWidthBits()); - int32_t division = (1 << order.getDivisionBits()); - - if (_ordering->getOrder() == document::OrderingSpecification::ASCENDING) { - uint64_t upperLimit = UINT64_MAX; - if (_ordering->getOrderingStart() < upperLimit - (width - division)) { - upperLimit = _ordering->getOrderingStart() + width - division; - } - if (order.getOrdering() >= _ordering->getOrderingStart() && - order.getOrdering() <= upperLimit) { - firstPass = true; - /*std::cerr << "First pass because ordering (+) " - << order.getOrdering() << " is between " - << _ordering->getOrderingStart() - << " and " << upperLimit << "\n";*/ - } else { - /*std::cerr << "Not first pass because ordering (+) " - << order.getOrdering() << " is not between " - << _ordering->getOrderingStart() - << " and " << upperLimit << "\n";*/ - } - } else { - uint64_t lowerLimit = 0; - if (_ordering->getOrderingStart() > (uint64_t)(width - division)) { - lowerLimit = _ordering->getOrderingStart() - (width - division); - } - if (order.getOrdering() <= _ordering->getOrderingStart() && - order.getOrdering() >= lowerLimit) { - firstPass = true; - /*std::cerr << "First pass because ordering (-) " - << order.getOrdering() << " is between " - << lowerLimit << " and " - << _ordering->getOrderingStart() << "\n";*/ - } else { - /*std::cerr << "Not first pass because ordering (-) " - << order.getOrdering() << " is not between " - << lowerLimit << " and " - << _ordering->getOrderingStart() << "\n";*/ - } - } - } else { -// std::cerr << "Not counting first pass: " << _ordering->getWidthBits() << "\n"; - } - if (firstPass) { _firstPassHits++; _firstPassBytes += size; @@ -579,7 +528,6 @@ Visitor::start(api::VisitorId id, api::StorageMessage::Id cmdId, framework::MicroSecTime toTimestamp, std::unique_ptr docSelection, const std::string& docSelectionString, - std::unique_ptr ordering, VisitorMessageHandler& handler, VisitorMessageSession::UP messageSession, documentapi::Priority::Value documentPriority) @@ -589,14 +537,13 @@ Visitor::start(api::VisitorId id, api::StorageMessage::Id cmdId, _visitorCmdId = cmdId; _id = name; _messageHandler = &handler; - _ordering = std::move(ordering); _documentSelection.reset(docSelection.release()); _documentSelectionString = docSelectionString; _buckets = buckets; _visitorOptions._fromTime = fromTimestamp; _visitorOptions._toTime = toTimestamp; _currentBucket = 0; - _hitCounter.reset(new HitCounter(_ordering.get())); + _hitCounter.reset(new HitCounter()); _messageSession = std::move(messageSession); _documentPriority = documentPriority; diff --git a/storage/src/vespa/storage/visiting/visitor.h b/storage/src/vespa/storage/visiting/visitor.h index f53ca5a60a0..88f3ad4f3c3 100644 --- a/storage/src/vespa/storage/visiting/visitor.h +++ b/storage/src/vespa/storage/visiting/visitor.h @@ -86,7 +86,7 @@ public: class HitCounter { public: - HitCounter(const document::OrderingSpecification* ordering); + HitCounter(); void addHit(const document::DocumentId& hit, uint32_t size); @@ -105,7 +105,6 @@ public: uint64_t _firstPassBytes; uint32_t _secondPassHits; uint64_t _secondPassBytes; - const document::OrderingSpecification* _ordering; }; enum VisitorState @@ -336,7 +335,6 @@ protected: std::unique_ptr _dataDestination; std::shared_ptr _documentSelection; std::string _documentSelectionString; - std::unique_ptr _ordering; vdslib::VisitorStatistics _visitorStatistics; bool isCompletedCalled() const { return _calledCompletedVisitor; } @@ -469,7 +467,6 @@ public: framework::MicroSecTime toTimestamp, std::unique_ptr docSelection, const std::string& docSelectionString, - std::unique_ptr, VisitorMessageHandler&, VisitorMessageSession::UP, documentapi::Priority::Value); diff --git a/storage/src/vespa/storage/visiting/visitorthread.cpp b/storage/src/vespa/storage/visiting/visitorthread.cpp index 1d33e829d49..142e7a89144 100644 --- a/storage/src/vespa/storage/visiting/visitorthread.cpp +++ b/storage/src/vespa/storage/visiting/visitorthread.cpp @@ -3,7 +3,6 @@ #include "visitorthread.h" #include "messages.h" #include -#include #include #include #include @@ -531,12 +530,6 @@ VisitorThread::onCreateVisitor( if (result.success()) { _visitors[cmd->getVisitorId()] = visitor; try{ - std::unique_ptr order; - if (docSelection.get()) { - document::OrderingSelector selector; - order = selector.select(*docSelection, - cmd->getVisitorOrdering()); - } VisitorMessageSession::UP messageSession( _messageSessionFactory.createSession(*visitor, *this)); documentapi::Priority::Value documentPriority = @@ -549,7 +542,6 @@ VisitorThread::onCreateVisitor( framework::MicroSecTime(cmd->getToTime()), std::move(docSelection), cmd->getDocumentSelection(), - std::move(order), _messageSender, std::move(messageSession), documentPriority); -- cgit v1.2.3