Merge pull request #19654 from vespa-engine/arnej/allow-discarding-issues

add forward_issues config option

10 files changed, 49 insertions, 8 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
index 921bf44d315..de8fa05f540 100644
--- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
+++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
@@ -101,6 +101,7 @@ public interface ModelContext {
         @ModelFeatureFlag(owners = {"bjorncs"}) default int maxConnectionLifeInHosted() { return 45; }
         @ModelFeatureFlag(owners = {"geirst", "vekterli"}) default int distributorMergeBusyWait() { return 10; }
         @ModelFeatureFlag(owners = {"vekterli", "geirst"}) default boolean distributorEnhancedMaintenanceScheduling() { return false; }
+        @ModelFeatureFlag(owners = {"arnej"}) default boolean forwardIssuesAsErrors() { return true; }
     }
 
     /** Warning: As elsewhere in this package, do not make backwards incompatible changes that will break old config models! */
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java
index 1200b7467b4..ff9db4a10bf 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java
@@ -68,6 +68,7 @@ public class ContentSearchCluster extends AbstractConfigProducer<SearchCluster>
     private final double defaultFeedConcurrency;
     private final double defaultDiskBloatFactor;
     private final int defaultDocStoreCompressionLevel;
+    private final boolean forwardIssuesToQrs;
 
     /** Whether the nodes of this cluster also hosts a container cluster in a hosted system */
     private final boolean combined;
@@ -210,11 +211,12 @@ public class ContentSearchCluster extends AbstractConfigProducer<SearchCluster>
         this.syncTransactionLog = syncTransactionLog;
 
         this.combined = combined;
-        feedSequencerType = convertFeedSequencerType(featureFlags.feedSequencerType());
-        feedTaskLimit = featureFlags.feedTaskLimit();
-        defaultFeedConcurrency = featureFlags.feedConcurrency();
-        defaultDocStoreCompressionLevel = featureFlags.docstoreCompressionLevel();
-        defaultDiskBloatFactor = featureFlags.diskBloatFactor();
+        this.feedSequencerType = convertFeedSequencerType(featureFlags.feedSequencerType());
+        this.feedTaskLimit = featureFlags.feedTaskLimit();
+        this.defaultFeedConcurrency = featureFlags.feedConcurrency();
+        this.defaultDiskBloatFactor = featureFlags.diskBloatFactor();
+        this.defaultDocStoreCompressionLevel = featureFlags.docstoreCompressionLevel();
+        this.forwardIssuesToQrs = featureFlags.forwardIssuesAsErrors();
     }
 
     public void setVisibilityDelay(double delay) {
@@ -411,6 +413,7 @@ public class ContentSearchCluster extends AbstractConfigProducer<SearchCluster>
         builder.flush.memory.each.diskbloatfactor(defaultDiskBloatFactor);
         builder.summary.log.chunk.compression.level(defaultDocStoreCompressionLevel);
         builder.summary.log.compact.compression.level(defaultDocStoreCompressionLevel);
+        builder.forward_issues(forwardIssuesToQrs);
 
         int numDocumentDbs = builder.documentdb.size();
         builder.initialize(new ProtonConfig.Initialize.Builder().threads(numDocumentDbs + 1));
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
index c299abcfb7c..89eb77f2250 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
@@ -196,6 +196,7 @@ public class ModelContextImpl implements ModelContext {
         private final int docstoreCompressionLevel;
         private final double diskBloatFactor;
         private final boolean distributorEnhancedMaintenanceScheduling;
+        private final boolean forwardIssuesAsErrors;
 
         public FeatureFlags(FlagSource source, ApplicationId appId) {
             this.defaultTermwiseLimit = flagValue(source, appId, Flags.DEFAULT_TERM_WISE_LIMIT);
@@ -230,6 +231,7 @@ public class ModelContextImpl implements ModelContext {
             this.docstoreCompressionLevel = flagValue(source, appId, Flags.DOCSTORE_COMPRESSION_LEVEL);
             this.diskBloatFactor = flagValue(source, appId, Flags.DISK_BLOAT_FACTOR);
             this.distributorEnhancedMaintenanceScheduling = flagValue(source, appId, Flags.DISTRIBUTOR_ENHANCED_MAINTENANCE_SCHEDULING);
+            this.forwardIssuesAsErrors = flagValue(source, appId, PermanentFlags.FORWARD_ISSUES_AS_ERRORS);
         }
 
         @Override public double defaultTermwiseLimit() { return defaultTermwiseLimit; }
@@ -266,6 +268,7 @@ public class ModelContextImpl implements ModelContext {
         @Override public double diskBloatFactor() { return diskBloatFactor; }
         @Override public int docstoreCompressionLevel() { return docstoreCompressionLevel; }
         @Override public boolean distributorEnhancedMaintenanceScheduling() { return distributorEnhancedMaintenanceScheduling; }
+        @Override public boolean forwardIssuesAsErrors() { return forwardIssuesAsErrors; }
 
         private static <V> V flagValue(FlagSource source, ApplicationId appId, UnboundFlag<? extends V, ?, ?> flag) {
             return flag.bindTo(source)
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java b/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java
index ae5317ecb53..2c0614805a9 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java
@@ -212,6 +212,13 @@ public class PermanentFlags {
             ZONE_ID
     );
 
+    public static final UnboundBooleanFlag FORWARD_ISSUES_AS_ERRORS = defineFeatureFlag(
+            "forward-issues-as-errors", true,
+            "When the backend detects a problematic issue with a query, it will by default send it as an error message to the QRS, which adds it in an ErrorHit in the result.  May be disabled using this flag.",
+            "Takes effect immediately",
+            ZONE_ID, APPLICATION_ID);
+
+
     private PermanentFlags() {}
 
     private static UnboundBooleanFlag defineFeatureFlag(
diff --git a/searchcore/src/vespa/searchcore/config/proton.def b/searchcore/src/vespa/searchcore/config/proton.def
index c1ec37e5662..e1bdf13fd36 100644
--- a/searchcore/src/vespa/searchcore/config/proton.def
+++ b/searchcore/src/vespa/searchcore/config/proton.def
@@ -514,3 +514,6 @@ bucketdb.checksumtype enum {LEGACY, XXHASH64} default = LEGACY restart
 ## FAST_VALUE uses the new and optimized FastValueBuilderFactory instead.
 ## TODO: Remove when default has been switched to FAST_VALUE.
 tensor_implementation enum {TENSOR_ENGINE, FAST_VALUE} default = FAST_VALUE
+
+## Whether to report issues back to the container via protobuf field
+forward_issues bool default = true
diff --git a/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.cpp b/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.cpp
index 3aedd952d1e..5ad4a7ed52b 100644
--- a/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.cpp
+++ b/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.cpp
@@ -46,6 +46,7 @@ MatchEngine::MatchEngine(size_t numThreads, size_t threadsPerSearch, uint32_t di
       _distributionKey(distributionKey),
       _async(async),
       _closed(false),
+      _forward_issues(true),
       _handlers(),
       _executor(std::max(size_t(1), numThreads / threadsPerSearch), 256_Ki, match_engine_executor),
       _threadBundlePool(std::max(size_t(1), threadsPerSearch)),
@@ -146,7 +147,13 @@ MatchEngine::performSearch(search::engine::SearchRequest::Source req)
         _threadBundlePool.release(std::move(threadBundle));
     }
     ret->request = req.release();
-    ret->my_issues = std::move(my_issues);
+    if (_forward_issues) {
+        ret->my_issues = std::move(my_issues);
+    } else {
+        my_issues->for_each_message([](const auto &msg){
+            LOG(warning, "unhandled issue: %s", msg.c_str());
+        });
+    }
     ret->setDistributionKey(_distributionKey);
     if ((ret->request->trace().getLevel() > 0) && ret->request->trace().hasTrace()) {
         ret->request->trace().getRoot().setLong("distribution-key", _distributionKey);
diff --git a/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.h b/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.h
index 74a39a3ec78..b4e32c45003 100644
--- a/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.h
+++ b/searchcore/src/vespa/searchcore/proton/matchengine/matchengine.h
@@ -21,6 +21,7 @@ private:
     const uint32_t                     _distributionKey;
     bool                               _async;
     bool                               _closed;
+    std::atomic<bool>                  _forward_issues;
     HandlerMap<ISearchHandler>         _handlers;
     vespalib::ThreadStackExecutor      _executor;
     vespalib::SimpleThreadBundle::Pool _threadBundlePool;
@@ -137,6 +138,8 @@ public:
             search::engine::SearchClient &client) override;
 
     void get_state(const vespalib::slime::Inserter &inserter, bool full) const override;
+
+    void set_issue_forwarding(bool enable) { _forward_issues = enable; }
 };
 
 } // namespace proton
diff --git a/searchcore/src/vespa/searchcore/proton/server/proton.cpp b/searchcore/src/vespa/searchcore/proton/server/proton.cpp
index edf68633124..116ce072700 100644
--- a/searchcore/src/vespa/searchcore/proton/server/proton.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/proton.cpp
@@ -296,8 +296,10 @@ Proton::init(const BootstrapConfig::SP & configSnapshot)
                                                  protonConfig.numthreadspersearch,
                                                  protonConfig.distributionkey,
                                                  protonConfig.search.async);
+    _matchEngine->set_issue_forwarding(protonConfig.forwardIssues);
     _distributionKey = protonConfig.distributionkey;
-    _summaryEngine= std::make_unique<SummaryEngine>(protonConfig.numsummarythreads, protonConfig.docsum.async);
+    _summaryEngine = std::make_unique<SummaryEngine>(protonConfig.numsummarythreads, protonConfig.docsum.async);
+    _summaryEngine->set_issue_forwarding(protonConfig.forwardIssues);
     _docsumBySlime = std::make_unique<DocsumBySlime>(*_summaryEngine);
 
     IFlushStrategy::SP strategy;
@@ -385,6 +387,8 @@ Proton::applyConfig(const BootstrapConfig::SP & configSnapshot)
     // Called by executor thread during reconfig.
     const ProtonConfig &protonConfig = configSnapshot->getProtonConfig();
     setFS4Compression(protonConfig);
+    _matchEngine->set_issue_forwarding(protonConfig.forwardIssues);
+    _summaryEngine->set_issue_forwarding(protonConfig.forwardIssues);
 
     _queryLimiter.configure(protonConfig.search.memory.limiter.maxthreads,
                             protonConfig.search.memory.limiter.mincoverage,
diff --git a/searchcore/src/vespa/searchcore/proton/summaryengine/summaryengine.cpp b/searchcore/src/vespa/searchcore/proton/summaryengine/summaryengine.cpp
index ec5c8c9b72d..95643c7d1a8 100644
--- a/searchcore/src/vespa/searchcore/proton/summaryengine/summaryengine.cpp
+++ b/searchcore/src/vespa/searchcore/proton/summaryengine/summaryengine.cpp
@@ -59,6 +59,7 @@ SummaryEngine::SummaryEngine(size_t numThreads, bool async)
     : _lock(),
       _async(async),
       _closed(false),
+      _forward_issues(true),
       _handlers(),
       _executor(numThreads, 128_Ki, summary_engine_executor),
       _metrics(std::make_unique<DocsumMetrics>())
@@ -145,7 +146,13 @@ SummaryEngine::getDocsums(DocsumRequest::UP req)
         reply = std::make_unique<DocsumReply>();
     }
     reply->setRequest(std::move(req));
-    reply->setIssues(std::move(my_issues));
+    if (_forward_issues) {
+        reply->setIssues(std::move(my_issues));
+    } else {
+        my_issues->for_each_message([](const auto &msg){
+            LOG(warning, "unhandled issue: %s", msg.c_str());
+        });
+    }
     return reply;
 }
 
diff --git a/searchcore/src/vespa/searchcore/proton/summaryengine/summaryengine.h b/searchcore/src/vespa/searchcore/proton/summaryengine/summaryengine.h
index 34eebdc839d..7f6d9328491 100644
--- a/searchcore/src/vespa/searchcore/proton/summaryengine/summaryengine.h
+++ b/searchcore/src/vespa/searchcore/proton/summaryengine/summaryengine.h
@@ -34,6 +34,7 @@ private:
     std::mutex                    _lock;
     bool                          _async;
     bool                          _closed;
+    std::atomic<bool>             _forward_issues;
     HandlerMap<ISearchHandler>    _handlers;
     vespalib::ThreadStackExecutor _executor;
     std::unique_ptr<metrics::MetricSet> _metrics;
@@ -126,6 +127,8 @@ public:
     DocsumReply::UP getDocsums(DocsumRequest::UP req) override;
 
     metrics::MetricSet & getMetrics() { return *_metrics; }
+
+    void set_issue_forwarding(bool enable) { _forward_issues = enable; }
 };
 
 } // namespace proton