Remove visitor ordering and order selection.

7 files changed, 25 insertions, 228 deletions

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 <vespa/storage/distributor/operations/external/visitororder.h>
 #include <vespa/storage/distributor/visitormetricsset.h>
 #include <vespa/document/base/exceptions.h>
-#include <vespa/document/select/orderingselector.h>
 #include <vespa/document/select/parser.h>
 #include <vespa/vespalib/stllike/asciistream.h>
 #include <iomanip>
@@ -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<document::BucketId>& buckets)
-{
-    if (buckets.size()) {
-        for (uint32_t i=0; i<buckets.size(); i++) {
-            if (isInconsistentlySplit(bucket, buckets[i])) {
-                return true;
-            }
-        }
-    }
-    return false;
-}
-
-} // End anonymous namespace
-
-bool
-VisitorOperation::isSpecialBucketForOrderDoc(const document::BucketId& bucketId) const
-{
-    if (isSplitPastOrderBits(bucketId, *_ordering)) {
-        LOG(spam, "Split past orderbits: Found in db: %s", bucketId.toString().c_str());
-    } else if (isInconsistentlySplit(bucketId, _superBucket.subBucketsVisitOrder)) {
-        LOG(spam, "Inconsistent: Found in db: %s", bucketId.toString().c_str());
-    } else {
-        return false;
-    }
-    return true;
-}
-
-std::vector<document::BucketId>::const_iterator
-VisitorOperation::addSpecialBucketsForOrderDoc(
-        std::vector<document::BucketId>::const_iterator iter,
-        std::vector<document::BucketId>::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<BucketDatabase::Entry>& buckets)
 {
@@ -427,7 +354,7 @@ VisitorOperation::pickBucketsToVisit(const std::vector<BucketDatabase::Entry>& b
         bucketVisitOrder.push_back(buckets[i].getBucketId());
     }
 
-    VisitorOrder bucketLessThan(*_ordering);
+    VisitorOrder bucketLessThan;
     std::sort(bucketVisitOrder.begin(), bucketVisitOrder.end(), bucketLessThan);
 
     std::vector<document::BucketId>::const_iterator iter(bucketVisitOrder.begin());
@@ -451,8 +378,6 @@ VisitorOperation::pickBucketsToVisit(const std::vector<BucketDatabase::Entry>& 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<uint16_t>& triedNodes,
-             uint16_t node)
+alreadyTried(const std::vector<uint16_t>& 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<document::select::Node> docSelection;
-    std::shared_ptr<const document::DocumentTypeRepo> 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<document::BucketId>::const_iterator addSpecialBucketsForOrderDoc(
-            std::vector<document::BucketId>::const_iterator iter,
-            std::vector<document::BucketId>::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<document::OrderingSpecification> _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<const document::OrderDocIdString&>(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<document::select::Node> docSelection,
                const std::string& docSelectionString,
-               std::unique_ptr<document::OrderingSpecification> 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<api::StorageMessageAddress> _dataDestination;
     std::shared_ptr<document::select::Node> _documentSelection;
     std::string _documentSelectionString;
-    std::unique_ptr<document::OrderingSpecification> _ordering;
     vdslib::VisitorStatistics _visitorStatistics;
 
     bool isCompletedCalled() const { return _calledCompletedVisitor; }
@@ -469,7 +467,6 @@ public:
                framework::MicroSecTime toTimestamp,
                std::unique_ptr<document::select::Node> docSelection,
                const std::string& docSelectionString,
-               std::unique_ptr<document::OrderingSpecification>,
                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 <vespa/document/select/bodyfielddetector.h>
-#include <vespa/document/select/orderingselector.h>
 #include <vespa/document/select/parser.h>
 #include <vespa/messagebus/rpcmessagebus.h>
 #include <vespa/storage/common/statusmessages.h>
@@ -531,12 +530,6 @@ VisitorThread::onCreateVisitor(
     if (result.success()) {
         _visitors[cmd->getVisitorId()] = visitor;
         try{
-            std::unique_ptr<document::OrderingSpecification> 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);