diff options
39 files changed, 841 insertions, 188 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 360968bacd5..92975e47f75 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 @@ -91,7 +91,6 @@ public interface ModelContext { @ModelFeatureFlag(owners = {"baldersheim"}) default int maxMergeQueueSize() { return 100; } @ModelFeatureFlag(owners = {"baldersheim"}) default boolean containerDumpHeapOnShutdownTimeout() { throw new UnsupportedOperationException("TODO specify default value"); } @ModelFeatureFlag(owners = {"baldersheim"}) default double containerShutdownTimeout() { throw new UnsupportedOperationException("TODO specify default value"); } - @ModelFeatureFlag(owners = {"geirst"}, removeAfter = "7.541") default boolean enableFeedBlockInDistributor() { return true; } @ModelFeatureFlag(owners = {"bjorncs", "tokle"}) default List<String> allowedAthenzProxyIdentities() { return List.of(); } @ModelFeatureFlag(owners = {"vekterli"}) default int maxActivationInhibitedOutOfSyncGroups() { return 0; } @ModelFeatureFlag(owners = {"hmusum"}) default String jvmOmitStackTraceInFastThrowOption(ClusterSpec.Type type) { return ""; } @@ -118,9 +117,7 @@ public interface ModelContext { @ModelFeatureFlag(owners = {"arnej"}) default boolean useQrserverServiceName() { return true; } @ModelFeatureFlag(owners = {"bjorncs", "baldersheim"}) default boolean enableJdiscPreshutdownCommand() { return true; } @ModelFeatureFlag(owners = {"arnej"}) default boolean avoidRenamingSummaryFeatures() { return false; } - @ModelFeatureFlag(owners = {"bjorncs", "baldersheim"}, removeAfter = "7.569") default boolean mergeGroupingResultInSearchInvoker() { return true; } @ModelFeatureFlag(owners = {"arnej"}) default boolean experimentalSdParsing() { return false; } - @ModelFeatureFlag(owners = {"hmusum"}, removeAfter = "7.571") default String adminClusterNodeArchitecture() { return adminClusterArchitecture().name(); } @ModelFeatureFlag(owners = {"hmusum"}) default Architecture adminClusterArchitecture() { return Architecture.getDefault(); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index 47519853ed0..ff1a4b6cc5f 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -1184,7 +1184,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { String message = "Invalid or misplaced JVM options in services.xml: " + String.join(",", invalidOptions) + "." + " See https://docs.vespa.ai/en/reference/services-container.html#jvm"; - if (failDeploymentWithInvalidJvmOptions) + if (failDeploymentWithInvalidJvmOptions && isHosted) throw new IllegalArgumentException(message); else logger.logApplicationPackage(WARNING, message); @@ -1250,7 +1250,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { String message = "Invalid or misplaced JVM GC options in services.xml: " + String.join(",", options) + "." + " See https://docs.vespa.ai/en/reference/services-container.html#jvm"; - if (failDeploymentWithInvalidJvmOptions) + if (failDeploymentWithInvalidJvmOptions && isHosted) throw new IllegalArgumentException(message); else logger.logApplicationPackage(WARNING, message); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Mail.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Mail.java index b586b97ddf0..36b6e251fbc 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Mail.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Mail.java @@ -5,6 +5,7 @@ import com.google.common.collect.ImmutableList; import java.util.Collection; import java.util.Objects; +import java.util.Optional; /** * A message with a subject and a nonempty set of recipients. @@ -16,18 +17,29 @@ public class Mail { private final Collection<String> recipients; private final String subject; private final String message; + private final Optional<String> htmlMessage; public Mail(Collection<String> recipients, String subject, String message) { + this(recipients, subject, message, Optional.empty()); + } + + public Mail(Collection<String> recipients, String subject, String message, String htmlMessage) { + this(recipients, subject, message, Optional.of(htmlMessage)); + } + + Mail(Collection<String> recipients, String subject, String message, Optional<String> htmlMessage) { if (recipients.isEmpty()) throw new IllegalArgumentException("Empty recipient list is not allowed."); recipients.forEach(Objects::requireNonNull); this.recipients = ImmutableList.copyOf(recipients); this.subject = Objects.requireNonNull(subject); this.message = Objects.requireNonNull(message); + this.htmlMessage = Objects.requireNonNull(htmlMessage); } public Collection<String> recipients() { return recipients; } public String subject() { return subject; } public String message() { return message; } + public Optional<String> htmlMessage() { return htmlMessage; } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index 91d127976ce..b0966f7db21 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -438,8 +438,15 @@ public class JobController { }); } finally { - for (Mutex lock : locks) - lock.close(); + for (Mutex lock : locks) { + try { + lock.close(); + } catch (Throwable t) { + log.log(WARNING, "Failed to close the lock " + lock + ": the lock may or may not " + + "have been released in ZooKeeper, and if not this controller " + + "must be restarted to release the lock", t); + } + } } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notify/Notifier.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notify/Notifier.java index 19d5cd8c1b3..6b14872b07d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notify/Notifier.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notify/Notifier.java @@ -95,14 +95,22 @@ public class Notifier { private Mail mailOf(Notification n, Collection<String> recipients) { var source = n.source(); - var subject = Text.format("[%s] %s Vespa Notification for %s - %s", n.level().toString().toUpperCase(), n.type().name(), source.tenant(), source.application()); + var subject = Text.format("[%s] %s Vespa Notification for %s", n.level().toString().toUpperCase(), n.type().name(), applicationIdSource(source)); var body = new StringBuilder(); body.append("Source: ").append(n.source().toString()).append("\n") .append("\n") .append(String.join("\n", n.messages())) .append("\n") .append(url(source).toString()); - return new Mail(recipients, subject.toString(), body.toString()); + return new Mail(recipients, subject, body.toString()); + } + + private String applicationIdSource(NotificationSource source) { + StringBuilder sb = new StringBuilder(); + sb.append(source.tenant().value()); + source.application().ifPresent(applicationName -> sb.append(".").append(applicationName.value())); + source.instance().ifPresent(instanceName -> sb.append(".").append(instanceName.value())); + return sb.toString(); } private URI url(NotificationSource source) { diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/RegistryCredentials.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/RegistryCredentials.java index 1ac61cd6c1f..29934e3d1aa 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/RegistryCredentials.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/RegistryCredentials.java @@ -10,16 +10,14 @@ import java.util.Objects; */ public class RegistryCredentials { - public static final RegistryCredentials none = new RegistryCredentials("", "", ""); + public static final RegistryCredentials none = new RegistryCredentials("", ""); private final String username; private final String password; - private final String registryAddress; - public RegistryCredentials(String username, String password, String registryAddress) { + public RegistryCredentials(String username, String password) { this.username = Objects.requireNonNull(username); this.password = Objects.requireNonNull(password); - this.registryAddress = Objects.requireNonNull(registryAddress); } public String username() { @@ -30,28 +28,23 @@ public class RegistryCredentials { return password; } - public String registryAddress() { - return registryAddress; - } - @Override public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; RegistryCredentials that = (RegistryCredentials) o; return username.equals(that.username) && - password.equals(that.password) && - registryAddress.equals(that.registryAddress); + password.equals(that.password); } @Override public int hashCode() { - return Objects.hash(username, password, registryAddress); + return Objects.hash(username, password); } @Override public String toString() { - return "registry credentials for " + registryAddress + " [username=" + username + ",password=" + password + "]"; + return "registry credentials [username=" + username + ",password=<hidden>]"; } } diff --git a/searchcore/src/tests/proton/matching/query_test.cpp b/searchcore/src/tests/proton/matching/query_test.cpp index 7f0232120e7..1c31cb1d1ad 100644 --- a/searchcore/src/tests/proton/matching/query_test.cpp +++ b/searchcore/src/tests/proton/matching/query_test.cpp @@ -1138,19 +1138,19 @@ Test::global_filter_is_calculated_and_handled() uint32_t docid_limit = 10; { // global filter is not wanted GlobalFilterBlueprint bp(result, false); - auto res = Query::handle_global_filter(bp, docid_limit, 0, 1); + auto res = Query::handle_global_filter(bp, docid_limit, 0, 1, nullptr); EXPECT_FALSE(res); EXPECT_FALSE(bp.filter); } { // estimated_hit_ratio < global_filter_lower_limit GlobalFilterBlueprint bp(result, true); - auto res = Query::handle_global_filter(bp, docid_limit, 0.31, 1); + auto res = Query::handle_global_filter(bp, docid_limit, 0.31, 1, nullptr); EXPECT_FALSE(res); EXPECT_FALSE(bp.filter); } { // estimated_hit_ratio <= global_filter_upper_limit GlobalFilterBlueprint bp(result, true); - auto res = Query::handle_global_filter(bp, docid_limit, 0, 0.3); + auto res = Query::handle_global_filter(bp, docid_limit, 0, 0.3, nullptr); EXPECT_TRUE(res); EXPECT_TRUE(bp.filter); EXPECT_TRUE(bp.filter->has_filter()); @@ -1163,7 +1163,7 @@ Test::global_filter_is_calculated_and_handled() } { // estimated_hit_ratio > global_filter_upper_limit GlobalFilterBlueprint bp(result, true); - auto res = Query::handle_global_filter(bp, docid_limit, 0, 0.29); + auto res = Query::handle_global_filter(bp, docid_limit, 0, 0.29, nullptr); EXPECT_TRUE(res); EXPECT_TRUE(bp.filter); EXPECT_FALSE(bp.filter->has_filter()); diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp index 3ff0a7d1808..624922eb27b 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_master.cpp @@ -109,7 +109,7 @@ MatchMaster::match(search::engine::Trace & trace, double match_time_s = 0.0; std::unique_ptr<vespalib::slime::Inserter> inserter; if (trace.shouldTrace(4)) { - inserter = std::make_unique<vespalib::slime::ArrayInserter>(trace.createCursor("match_threads").setArray("threads")); + inserter = std::make_unique<vespalib::slime::ArrayInserter>(trace.createCursor("query_execution").setArray("threads")); } for (size_t i = 0; i < threadState.size(); ++i) { const MatchThread & matchThread = *threadState[i]; diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp index 37635825295..3d8d56f0150 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp @@ -3,12 +3,15 @@ #include "match_tools.h" #include "querynodes.h" #include <vespa/searchcorespi/index/indexsearchable.h> +#include <vespa/searchlib/attribute/attribute_blueprint_params.h> +#include <vespa/searchlib/attribute/attribute_operation.h> +#include <vespa/searchlib/attribute/diversity.h> +#include <vespa/searchlib/engine/trace.h> #include <vespa/searchlib/fef/indexproperties.h> #include <vespa/searchlib/fef/ranksetup.h> -#include <vespa/searchlib/engine/trace.h> -#include <vespa/searchlib/attribute/diversity.h> -#include <vespa/searchlib/attribute/attribute_operation.h> -#include <vespa/searchlib/attribute/attribute_blueprint_params.h> +#include <vespa/vespalib/data/slime/cursor.h> +#include <vespa/vespalib/data/slime/inject.h> +#include <vespa/vespalib/data/slime/inserter.h> #include <vespa/vespalib/util/issue.h> #include <vespa/log/log.h> @@ -167,7 +170,7 @@ MatchToolsFactory(QueryLimiter & queryLimiter, const vespalib::Doom & doom, ISearchContext & searchContext, IAttributeContext & attributeContext, - search::engine::Trace & trace, + search::engine::Trace & root_trace, vespalib::stringref queryStack, const vespalib::string & location, const ViewResolver & viewResolver, @@ -188,34 +191,35 @@ MatchToolsFactory(QueryLimiter & queryLimiter, _diversityParams(), _valid(false) { - trace.addEvent(4, "MTF: Start"); + search::engine::Trace trace(root_trace.getRelativeTime(), root_trace.getLevel()); + trace.addEvent(4, "Start query setup"); _query.setWhiteListBlueprint(metaStore.createWhiteListBlueprint()); - trace.addEvent(5, "MTF: Build query"); + trace.addEvent(5, "Deserialize and build query tree"); _valid = _query.buildTree(queryStack, location, viewResolver, indexEnv, rankSetup.split_unpacking_iterators(), rankSetup.delay_unpacking_iterators()); if (_valid) { _query.extractTerms(_queryEnv.terms()); _query.extractLocations(_queryEnv.locations()); - trace.addEvent(5, "MTF: reserve handles"); + trace.addEvent(5, "Build query execution plan"); _query.reserveHandles(_requestContext, searchContext, _mdl); + trace.addEvent(5, "Optimize query execution plan"); _query.optimize(); - trace.addEvent(4, "MTF: Fetch Postings"); + trace.addEvent(4, "Perform dictionary lookups and posting lists initialization"); _query.fetchPostings(); if (is_search) { - trace.addEvent(5, "MTF: Handle Global Filters"); double lower_limit = GlobalFilterLowerLimit::lookup(rankProperties, rankSetup.get_global_filter_lower_limit()); double upper_limit = GlobalFilterUpperLimit::lookup(rankProperties, rankSetup.get_global_filter_upper_limit()); - _query.handle_global_filter(searchContext.getDocIdLimit(), lower_limit, upper_limit); + _query.handle_global_filter(searchContext.getDocIdLimit(), lower_limit, upper_limit, trace); } _query.freeze(); - trace.addEvent(5, "MTF: prepareSharedState"); + trace.addEvent(5, "Prepare shared state for multi-threaded rank executors"); _rankSetup.prepareSharedState(_queryEnv, _queryEnv.getObjectStore()); _diversityParams = extractDiversityParams(_rankSetup, rankProperties); DegradationParams degradationParams = extractDegradationParams(_rankSetup, rankProperties); if (degradationParams.enabled()) { - trace.addEvent(5, "MTF: Build MatchPhaseLimiter"); + trace.addEvent(5, "Setup match phase limiter"); _match_limiter = std::make_unique<MatchPhaseLimiter>(metaStore.getCommittedDocIdLimit(), searchContext.getAttributes(), _requestContext, degradationParams, _diversityParams); } @@ -223,7 +227,11 @@ MatchToolsFactory(QueryLimiter & queryLimiter, if ( ! _match_limiter) { _match_limiter = std::make_unique<NoMatchPhaseLimiter>(); } - trace.addEvent(4, "MTF: Complete"); + trace.addEvent(4, "Complete query setup"); + if (root_trace.shouldTrace(4)) { + vespalib::slime::ObjectInserter inserter(root_trace.createCursor("query_setup"), "traces"); + vespalib::slime::inject(trace.getTraces(), inserter); + } } MatchToolsFactory::~MatchToolsFactory() = default; diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_tools.h b/searchcore/src/vespa/searchcore/proton/matching/match_tools.h index 4b5f9cf76cc..a7d39a0c3e8 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_tools.h +++ b/searchcore/src/vespa/searchcore/proton/matching/match_tools.h @@ -114,7 +114,7 @@ public: const vespalib::Doom & softDoom, ISearchContext &searchContext, search::attribute::IAttributeContext &attributeContext, - search::engine::Trace & trace, + search::engine::Trace & root_trace, vespalib::stringref queryStack, const vespalib::string &location, const ViewResolver &viewResolver, diff --git a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp index e945bbb850b..756af216988 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp @@ -176,7 +176,7 @@ namespace { void traceQuery(uint32_t traceLevel, Trace & trace, const Query & query) { if (traceLevel <= trace.getLevel()) { if (query.peekRoot()) { - vespalib::slime::ObjectInserter inserter(trace.createCursor("blueprint"), "optimized"); + vespalib::slime::ObjectInserter inserter(trace.createCursor("query_execution_plan"), "optimized"); query.peekRoot()->asSlime(inserter); } } diff --git a/searchcore/src/vespa/searchcore/proton/matching/query.cpp b/searchcore/src/vespa/searchcore/proton/matching/query.cpp index 95fe846a088..84671ec72c7 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/query.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/query.cpp @@ -1,15 +1,16 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include "query.h" #include "blueprintbuilder.h" #include "matchdatareservevisitor.h" +#include "query.h" #include "resolveviewvisitor.h" -#include "termdataextractor.h" #include "sameelementmodifier.h" +#include "termdataextractor.h" #include "unpacking_iterators_optimizer.h" #include <vespa/document/datatype/positiondatatype.h> -#include <vespa/searchlib/common/geo_location_spec.h> #include <vespa/searchlib/common/geo_location_parser.h> +#include <vespa/searchlib/common/geo_location_spec.h> +#include <vespa/searchlib/engine/trace.h> #include <vespa/searchlib/parsequery/stackdumpiterator.h> #include <vespa/searchlib/queryeval/intermediate_blueprints.h> #include <vespa/vespalib/util/issue.h> @@ -27,19 +28,19 @@ using search::fef::IIndexEnvironment; using search::fef::ITermData; using search::fef::MatchData; using search::fef::MatchDataLayout; +using search::query::LocationTerm; using search::query::Node; using search::query::QueryTreeCreator; using search::query::Weight; using search::queryeval::AndBlueprint; using search::queryeval::AndNotBlueprint; -using search::queryeval::RankBlueprint; -using search::queryeval::IntermediateBlueprint; using search::queryeval::Blueprint; using search::queryeval::IRequestContext; +using search::queryeval::IntermediateBlueprint; +using search::queryeval::RankBlueprint; using search::queryeval::SearchIterator; -using search::query::LocationTerm; -using vespalib::string; using vespalib::Issue; +using vespalib::string; using std::vector; namespace proton::matching { @@ -244,12 +245,14 @@ Query::fetchPostings() } void -Query::handle_global_filter(uint32_t docid_limit, double global_filter_lower_limit, double global_filter_upper_limit) +Query::handle_global_filter(uint32_t docid_limit, double global_filter_lower_limit, double global_filter_upper_limit, + search::engine::Trace& trace) { - if (!handle_global_filter(*_blueprint, docid_limit, global_filter_lower_limit, global_filter_upper_limit)) { + if (!handle_global_filter(*_blueprint, docid_limit, global_filter_lower_limit, global_filter_upper_limit, &trace)) { return; } // optimized order may change after accounting for global filter: + trace.addEvent(5, "Optimize query execution plan to account for global filter"); _blueprint = Blueprint::optimize(std::move(_blueprint)); LOG(debug, "blueprint after handle_global_filter:\n%s\n", _blueprint->asString().c_str()); // strictness may change if optimized order changed: @@ -257,7 +260,9 @@ Query::handle_global_filter(uint32_t docid_limit, double global_filter_lower_lim } bool -Query::handle_global_filter(Blueprint& blueprint, uint32_t docid_limit, double global_filter_lower_limit, double global_filter_upper_limit) +Query::handle_global_filter(Blueprint& blueprint, uint32_t docid_limit, + double global_filter_lower_limit, double global_filter_upper_limit, + search::engine::Trace* trace) { using search::queryeval::GlobalFilter; double estimated_hit_ratio = blueprint.getState().hit_ratio(docid_limit); @@ -265,24 +270,37 @@ Query::handle_global_filter(Blueprint& blueprint, uint32_t docid_limit, double g return false; } - LOG(debug, "docid_limit=%d, estimated_hit_ratio=%1.2f, global_filter_lower_limit=%1.2f, global_filter_upper_limit=%1.2f", - docid_limit, estimated_hit_ratio, global_filter_lower_limit, global_filter_upper_limit); if (estimated_hit_ratio < global_filter_lower_limit) { + if (trace && trace->shouldTrace(5)) { + trace->addEvent(5, vespalib::make_string("Skip calculate global filter (estimated_hit_ratio (%f) < lower_limit (%f))", + estimated_hit_ratio, global_filter_lower_limit)); + } return false; } + std::shared_ptr<GlobalFilter> global_filter; if (estimated_hit_ratio <= global_filter_upper_limit) { + if (trace && trace->shouldTrace(5)) { + trace->addEvent(5, vespalib::make_string("Calculate global filter (estimated_hit_ratio (%f) <= upper_limit (%f))", + estimated_hit_ratio, global_filter_upper_limit)); + } auto constraint = Blueprint::FilterConstraint::UPPER_BOUND; bool strict = true; auto filter_iterator = blueprint.createFilterSearch(strict, constraint); filter_iterator->initRange(1, docid_limit); auto white_list = filter_iterator->get_hits(1); - auto global_filter = GlobalFilter::create(std::move(white_list)); - blueprint.set_global_filter(*global_filter); + global_filter = GlobalFilter::create(std::move(white_list)); } else { - auto no_filter = GlobalFilter::create(); - blueprint.set_global_filter(*no_filter); + if (trace && trace->shouldTrace(5)) { + trace->addEvent(5, vespalib::make_string("Create match all global filter (estimated_hit_ratio (%f) > upper_limit (%f))", + estimated_hit_ratio, global_filter_upper_limit)); + } + global_filter = GlobalFilter::create(); + } + if (trace) { + trace->addEvent(5, "Handle global filter in query execution plan"); } + blueprint.set_global_filter(*global_filter); return true; } diff --git a/searchcore/src/vespa/searchcore/proton/matching/query.h b/searchcore/src/vespa/searchcore/proton/matching/query.h index 29bca310502..09eaed5c4a9 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/query.h +++ b/searchcore/src/vespa/searchcore/proton/matching/query.h @@ -10,6 +10,8 @@ #include <vespa/searchlib/queryeval/blueprint.h> #include <vespa/searchlib/queryeval/irequestcontext.h> +namespace search::engine { class Trace; } + namespace proton::matching { class ViewResolver; @@ -93,7 +95,8 @@ public: void optimize(); void fetchPostings(); - void handle_global_filter(uint32_t docid_limit, double global_filter_lower_limit, double global_filter_upper_limit); + void handle_global_filter(uint32_t docid_limit, double global_filter_lower_limit, double global_filter_upper_limit, + search::engine::Trace& trace); /** * Calculates and handles the global filter if needed by the blueprint tree. @@ -109,7 +112,8 @@ public: * @return whether the global filter was set on the blueprint. */ static bool handle_global_filter(Blueprint& blueprint, uint32_t docid_limit, - double global_filter_lower_limit, double global_filter_upper_limit); + double global_filter_lower_limit, double global_filter_upper_limit, + search::engine::Trace* trace); void freeze(); diff --git a/searchlib/src/vespa/searchlib/engine/trace.h b/searchlib/src/vespa/searchlib/engine/trace.h index 1940af5cf38..709dc05d93c 100644 --- a/searchlib/src/vespa/searchlib/engine/trace.h +++ b/searchlib/src/vespa/searchlib/engine/trace.h @@ -85,6 +85,7 @@ public: vespalib::string toString() const; bool hasTrace() const { return static_cast<bool>(_trace); } Cursor & getRoot() const { return root(); } + Cursor & getTraces() const { return traces(); } vespalib::Slime & getSlime() const { return slime(); } bool shouldTrace(uint32_t level) const { return level <= _level; } uint32_t getLevel() const { return _level; } diff --git a/searchsummary/src/tests/docsummary/attributedfw/attributedfw_test.cpp b/searchsummary/src/tests/docsummary/attributedfw/attributedfw_test.cpp index 9f6ca8f6b57..67d505582d8 100644 --- a/searchsummary/src/tests/docsummary/attributedfw/attributedfw_test.cpp +++ b/searchsummary/src/tests/docsummary/attributedfw/attributedfw_test.cpp @@ -59,6 +59,8 @@ public: } _writer = AttributeDFWFactory::create(_attrs.mgr(), field_name, filter_elements, _matching_elems_fields); _writer->setIndex(0); + EXPECT_TRUE(_writer->setFieldWriterStateIndex(0)); + _state._fieldWriterStates.resize(1); _field_name = field_name; _state._attributes.resize(1); _state._attributes[0] = _state._attrCtx->getAttribute(field_name); @@ -77,6 +79,7 @@ public: _callback.clear(); _callback.add_matching_elements(docid, _field_name, matching_elems); _state._matching_elements = std::unique_ptr<MatchingElements>(); + _state._fieldWriterStates[0] = nullptr; // Force new state to pick up changed matching elements expect_field(exp_slime_as_json, docid); } }; diff --git a/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp index 448feedac80..eb4f1a19e06 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp @@ -3,12 +3,12 @@ #include "attributedfw.h" #include "docsumstate.h" #include "docsumwriter.h" +#include "docsum_field_writer_state.h" #include <vespa/eval/eval/value.h> #include <vespa/eval/eval/value_codec.h> -#include <vespa/searchcommon/attribute/iattributecontext.h> +#include <vespa/searchcommon/attribute/i_multi_value_attribute.h> +#include <vespa/searchcommon/attribute/multi_value_traits.h> #include <vespa/searchlib/attribute/iattributemanager.h> -#include <vespa/searchlib/attribute/integerbase.h> -#include <vespa/searchlib/attribute/stringbase.h> #include <vespa/searchlib/common/matching_elements.h> #include <vespa/searchlib/common/matching_elements_fields.h> #include <vespa/searchlib/tensor/i_tensor_attribute.h> @@ -23,6 +23,8 @@ using namespace search; using search::attribute::BasicType; using search::attribute::IAttributeContext; using search::attribute::IAttributeVector; +using search::attribute::IMultiValueAttribute; +using search::attribute::IMultiValueReadView; using vespalib::Memory; using vespalib::slime::Cursor; using vespalib::slime::Inserter; @@ -140,145 +142,219 @@ SingleAttrDFW::insertField(uint32_t docid, GetDocsumsState * state, ResType type //----------------------------------------------------------------------------- -template <typename DataType> -class MultiAttrDFW : public AttrDFW { -private: - bool _is_weighted_set; - bool _filter_elements; - std::shared_ptr<MatchingElementsFields> _matching_elems_fields; +template <typename MultiValueType> +const IMultiValueReadView<MultiValueType>* +make_read_view(const IAttributeVector& attribute, vespalib::Stash& stash) +{ + auto multi_value_attribute = attribute.as_multi_value_attribute(); + if (multi_value_attribute != nullptr) { + return multi_value_attribute->make_read_view(IMultiValueAttribute::MultiValueTag<MultiValueType>(), stash); + } + return nullptr; +} +class EmptyWriterState : public DocsumFieldWriterState +{ public: - explicit MultiAttrDFW(const vespalib::string& attr_name, bool is_weighted_set, - bool filter_elements, std::shared_ptr<MatchingElementsFields> matching_elems_fields) - : AttrDFW(attr_name), - _is_weighted_set(is_weighted_set), - _filter_elements(filter_elements), - _matching_elems_fields(std::move(matching_elems_fields)) - { - if (filter_elements && _matching_elems_fields) { - _matching_elems_fields->add_field(attr_name); - } - } - void insertField(uint32_t docid, GetDocsumsState* state, ResType type, Inserter& target) override; + EmptyWriterState() = default; + ~EmptyWriterState() = default; + void insertField(uint32_t, Inserter&) override { } }; -void -set(const vespalib::string & value, Symbol itemSymbol, Cursor & cursor) +template <typename MultiValueType> +class MultiAttrDFWState : public DocsumFieldWriterState { - cursor.setString(itemSymbol, value); -} + const vespalib::string& _field_name; + const IMultiValueReadView<MultiValueType>* _read_view; + const MatchingElements* _matching_elements; +public: + MultiAttrDFWState(const vespalib::string& field_name, const IAttributeVector& attr, vespalib::Stash& stash, const MatchingElements* matching_elements); + ~MultiAttrDFWState() override; + void insertField(uint32_t docid, Inserter& target) override; +}; -void -append(const IAttributeVector::WeightedString & element, Cursor& arr) -{ - arr.addString(element.getValue()); -} -void -set(int64_t value, Symbol itemSymbol, Cursor & cursor) +template <typename MultiValueType> +MultiAttrDFWState<MultiValueType>::MultiAttrDFWState(const vespalib::string& field_name, const IAttributeVector& attr, vespalib::Stash& stash, const MatchingElements* matching_elements) + : _field_name(field_name), + _read_view(make_read_view<MultiValueType>(attr, stash)), + _matching_elements(matching_elements) { - cursor.setLong(itemSymbol, value); } -void -append(const IAttributeVector::WeightedInt & element, Cursor& arr) -{ - arr.addLong(element.getValue()); -} +template <typename MultiValueType> +MultiAttrDFWState<MultiValueType>::~MultiAttrDFWState() = default; +template <typename V> void -set(double value, Symbol itemSymbol, Cursor & cursor) +set_value(V value, Symbol item_symbol, Cursor& cursor) { - cursor.setDouble(itemSymbol, value); + if constexpr (std::is_same_v<V, const char*>) { + cursor.setString(item_symbol, value); + } else if constexpr(std::is_floating_point_v<V>) { + cursor.setDouble(item_symbol, value); + } else { + cursor.setLong(item_symbol, value); + } } +template <typename V> void -append(const IAttributeVector::WeightedFloat & element, Cursor& arr) +append_value(V value, Cursor& arr) { - arr.addDouble(element.getValue()); + if constexpr (std::is_same_v<V, const char*>) { + arr.addString(value); + } else if constexpr(std::is_floating_point_v<V>) { + arr.addDouble(value); + } else { + arr.addLong(value); + } } Memory ITEM("item"); Memory WEIGHT("weight"); -template <typename DataType> +template <typename MultiValueType> void -MultiAttrDFW<DataType>::insertField(uint32_t docid, GetDocsumsState* state, ResType, Inserter& target) +MultiAttrDFWState<MultiValueType>::insertField(uint32_t docid, Inserter& target) { - const auto& attr = get_attribute(*state); - uint32_t entries = attr.getValueCount(docid); - if (entries == 0) { - return; // Don't insert empty fields + using ValueType = multivalue::ValueType_t<MultiValueType>; + if (!_read_view) { + return; } + auto elements = _read_view->get_values(docid); + if (elements.empty()) { + return; + } + Cursor &arr = target.insertArray(elements.size()); - std::vector<DataType> elements(entries); - entries = std::min(entries, attr.get(docid, elements.data(), entries)); - Cursor &arr = target.insertArray(entries); - - if (_filter_elements) { - const auto& matching_elems = state->get_matching_elements(*_matching_elems_fields) - .get_matching_elements(docid, getAttributeName()); - if (!matching_elems.empty() && matching_elems.back() < entries) { - if (_is_weighted_set) { + if (_matching_elements) { + const auto& matching_elems = _matching_elements->get_matching_elements(docid, _field_name); + if (!matching_elems.empty() && matching_elems.back() < elements.size()) { + if constexpr (multivalue::is_WeightedValue_v<MultiValueType>) { Symbol itemSymbol = arr.resolve(ITEM); Symbol weightSymbol = arr.resolve(WEIGHT); for (uint32_t id_to_keep : matching_elems) { - const DataType & element = elements[id_to_keep]; + auto& element = elements[id_to_keep]; Cursor& elemC = arr.addObject(); - set(element.getValue(), itemSymbol, elemC); - elemC.setLong(weightSymbol, element.getWeight()); + set_value<ValueType>(element.value(), itemSymbol, elemC); + elemC.setLong(weightSymbol, element.weight()); } } else { for (uint32_t id_to_keep : matching_elems) { - append(elements[id_to_keep], arr); + append_value<ValueType>(elements[id_to_keep], arr); } } } } else { - if (_is_weighted_set) { + if constexpr (multivalue::is_WeightedValue_v<MultiValueType>) { Symbol itemSymbol = arr.resolve(ITEM); Symbol weightSymbol = arr.resolve(WEIGHT); for (const auto & element : elements) { Cursor& elemC = arr.addObject(); - set(element.getValue(), itemSymbol, elemC); - elemC.setLong(weightSymbol, element.getWeight()); + set_value<ValueType>(element.value(), itemSymbol, elemC); + elemC.setLong(weightSymbol, element.weight()); } } else { for (const auto & element : elements) { - append(element, arr); + append_value<ValueType>(element, arr); } } } } -std::unique_ptr<IDocsumFieldWriter> -create_multi_writer(const IAttributeVector& attr, - bool filter_elements, - std::shared_ptr<MatchingElementsFields> matching_elems_fields) +class MultiAttrDFW : public AttrDFW { +private: + bool _filter_elements; + uint32_t _state_index; // index into _fieldWriterStates in GetDocsumsState + std::shared_ptr<MatchingElementsFields> _matching_elems_fields; + +public: + explicit MultiAttrDFW(const vespalib::string& attr_name, bool filter_elements, std::shared_ptr<MatchingElementsFields> matching_elems_fields) + : AttrDFW(attr_name), + _filter_elements(filter_elements), + _matching_elems_fields(std::move(matching_elems_fields)) + { + if (filter_elements && _matching_elems_fields) { + _matching_elems_fields->add_field(attr_name); + } + } + bool setFieldWriterStateIndex(uint32_t fieldWriterStateIndex) override; + void insertField(uint32_t docid, GetDocsumsState* state, ResType type, Inserter& target) override; +}; + +bool +MultiAttrDFW::setFieldWriterStateIndex(uint32_t fieldWriterStateIndex) +{ + _state_index = fieldWriterStateIndex; + return true; +} + +template <typename DataType> +DocsumFieldWriterState* +make_field_writer_state_helper(const vespalib::string& field_name, const IAttributeVector& attr, vespalib::Stash& stash, const MatchingElements* matching_elements) { - auto type = attr.getBasicType(); bool is_weighted_set = attr.hasWeightedSetType(); + if (is_weighted_set) { + return &stash.create<MultiAttrDFWState<multivalue::WeightedValue<DataType>>>(field_name, attr, stash, matching_elements); + } else { + return &stash.create<MultiAttrDFWState<DataType>>(field_name, attr, stash, matching_elements); + } +} + +DocsumFieldWriterState* +make_field_writer_state(const vespalib::string& field_name, const IAttributeVector& attr, vespalib::Stash& stash, const MatchingElements* matching_elements) +{ + auto type = attr.getBasicType(); switch (type) { - case BasicType::NONE: - case BasicType::STRING: { - return std::make_unique<MultiAttrDFW<IAttributeVector::WeightedString>>(attr.getName(), is_weighted_set, - filter_elements, std::move(matching_elems_fields)); + case BasicType::Type::STRING: + return make_field_writer_state_helper<const char*>(field_name, attr, stash, matching_elements); + case BasicType::Type::INT8: + return make_field_writer_state_helper<int8_t>(field_name, attr, stash, matching_elements); + case BasicType::Type::INT16: + return make_field_writer_state_helper<int16_t>(field_name, attr, stash, matching_elements); + case BasicType::Type::INT32: + return make_field_writer_state_helper<int32_t>(field_name, attr, stash, matching_elements); + case BasicType::Type::INT64: + return make_field_writer_state_helper<int64_t>(field_name, attr, stash, matching_elements); + case BasicType::Type::FLOAT: + return make_field_writer_state_helper<float>(field_name, attr, stash, matching_elements); + case BasicType::Type::DOUBLE: + return make_field_writer_state_helper<double>(field_name, attr, stash, matching_elements); + default: + ; + } + return &stash.create<EmptyWriterState>(); +} + +void +MultiAttrDFW::insertField(uint32_t docid, GetDocsumsState *state, ResType, vespalib::slime::Inserter &target) +{ + auto& field_writer_state = state->_fieldWriterStates[_state_index]; + if (!field_writer_state) { + const MatchingElements *matching_elements = nullptr; + if (_filter_elements) { + matching_elements = &state->get_matching_elements(*_matching_elems_fields); + } + const auto& attr = get_attribute(*state); + field_writer_state = make_field_writer_state(getAttributeName(), attr, state->get_stash(), matching_elements); } - case BasicType::BOOL: - case BasicType::UINT2: - case BasicType::UINT4: + field_writer_state->insertField(docid, target); +} + +std::unique_ptr<IDocsumFieldWriter> +create_multi_writer(const IAttributeVector& attr, bool filter_elements, std::shared_ptr<MatchingElementsFields> matching_elems_fields) +{ + auto type = attr.getBasicType(); + switch (type) { + case BasicType::STRING: case BasicType::INT8: case BasicType::INT16: case BasicType::INT32: - case BasicType::INT64: { - return std::make_unique<MultiAttrDFW<IAttributeVector::WeightedInt>>(attr.getName(), is_weighted_set, - filter_elements, std::move(matching_elems_fields)); - } + case BasicType::INT64: case BasicType::FLOAT: - case BasicType::DOUBLE: { - return std::make_unique<MultiAttrDFW<IAttributeVector::WeightedFloat>>(attr.getName(), is_weighted_set, - filter_elements, std::move(matching_elems_fields)); - } + case BasicType::DOUBLE: + return std::make_unique<MultiAttrDFW>(attr.getName(), filter_elements, std::move(matching_elems_fields)); default: // should not happen LOG(error, "Bad value for attribute type: %u", type); diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.h b/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.h index 1127af6d6bd..c25aca15200 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.h +++ b/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.h @@ -92,7 +92,7 @@ public: // used by RankFeaturesDFW FeatureSet::SP _rankFeatures; - // Used by AttributeCombinerDFW when filtering is enabled + // Used by AttributeCombinerDFW and MultiAttrDFW when filtering is enabled std::unique_ptr<search::MatchingElements> _matching_elements; GetDocsumsState(const GetDocsumsState &) = delete; diff --git a/security-utils/pom.xml b/security-utils/pom.xml index ac856dff6c3..5c4deecc437 100644 --- a/security-utils/pom.xml +++ b/security-utils/pom.xml @@ -96,7 +96,7 @@ <configuration> <instructions> <Bundle-SymbolicName>${project.artifactId}</Bundle-SymbolicName> - <Bundle-Version>${parsedVersion.majorVersion}.${parsedVersion.minorVersion}.${parsedVersion.incrementalVersion}</Bundle-Version> + <Bundle-Version>7.0.0</Bundle-Version> <Export-Package>com.yahoo.security.*;version=1.0.0;-noimport:=true</Export-Package> <_nouses>true</_nouses> <!-- Don't include 'uses' directives for package exports --> <_fixupmessages>"Classes found in the wrong directory"</_fixupmessages> <!-- Hide warnings for bouncycastle multi-release jars --> diff --git a/vespajlib/src/main/java/com/yahoo/io/NativeIO.java b/vespajlib/src/main/java/com/yahoo/io/NativeIO.java index 28c3f21b24c..af101211b4a 100644 --- a/vespajlib/src/main/java/com/yahoo/io/NativeIO.java +++ b/vespajlib/src/main/java/com/yahoo/io/NativeIO.java @@ -1,6 +1,8 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.io; +import com.yahoo.nativec.PosixFAdvise; + import java.io.File; import java.io.FileDescriptor; import java.io.FileInputStream; @@ -10,17 +12,12 @@ import java.io.SyncFailedException; import java.lang.reflect.Field; import java.util.logging.Logger; -import com.sun.jna.LastErrorException; -import com.sun.jna.Native; -import com.sun.jna.Platform; - /** * Provides functionality only possible through native C library. */ public class NativeIO { private static final Logger logger = Logger.getLogger(NativeIO.class.getName()); private static final String DISABLE_NATIVE_IO = "DISABLE_NATIVE_IO"; - private static final int POSIX_FADV_DONTNEED = 4; // See /usr/include/linux/fadvise.h private static final InitResult fdField = new InitResult(); private static class InitResult { private final boolean initialized; @@ -31,17 +28,14 @@ public class NativeIO { boolean initComplete = false; boolean disabled = true; Field field = null; - Throwable exception = null; + Throwable exception = PosixFAdvise.init(); try { - if (Platform.isLinux()) { + if (exception == null) { disabled = System.getenv().containsKey(DISABLE_NATIVE_IO); if (!disabled) { - Native.register(Platform.C_LIBRARY_NAME); field = getField(FileDescriptor.class, "fd"); initComplete = true; } - } else { - exception = new RuntimeException("Platform is unsúpported. Only supported on linux."); } } catch (Throwable throwable) { exception = throwable; @@ -68,14 +62,12 @@ public class NativeIO { } } - private static native int posix_fadvise(int fd, long offset, long len, int flag) throws LastErrorException; - public NativeIO() { if ( ! fdField.isInitialized()) { if (fdField.isEnabled()) { logger.warning("Native IO not possible due to " + getError().getMessage()); } else { - logger.info("Native IO has been disable explicit via system property " + DISABLE_NATIVE_IO); + logger.info("Native IO has been disabled explicit via system property " + DISABLE_NATIVE_IO); } } } @@ -96,7 +88,7 @@ public class NativeIO { } } if (valid()) { - posix_fadvise(fdField.getNativeFD(fd), offset, len, POSIX_FADV_DONTNEED); + PosixFAdvise.posix_fadvise(fdField.getNativeFD(fd), offset, len, PosixFAdvise.POSIX_FADV_DONTNEED); } } /** diff --git a/vespajlib/src/main/java/com/yahoo/nativec/GLibcVersion.java b/vespajlib/src/main/java/com/yahoo/nativec/GLibcVersion.java new file mode 100644 index 00000000000..67ae30c84f5 --- /dev/null +++ b/vespajlib/src/main/java/com/yahoo/nativec/GLibcVersion.java @@ -0,0 +1,21 @@ +package com.yahoo.nativec; + +public class GLibcVersion { + private final static Throwable initException = NativeC.loadLibrary(GLibcVersion.class); + public static Throwable init() { + return initException; + } + private final String version; + private final int major; + private final int minor; + public GLibcVersion() { + version = gnu_get_libc_version(); + String [] parts = version.split("\\."); + major = parts.length > 0 ? Integer.valueOf(parts[0]) : -1; + minor = parts.length > 1 ? Integer.valueOf(parts[1]) : -1; + } + private native static String gnu_get_libc_version(); + public String version() { return version; } + public int major() { return major; } + public int minor() { return minor; } +} diff --git a/vespajlib/src/main/java/com/yahoo/nativec/MallInfo.java b/vespajlib/src/main/java/com/yahoo/nativec/MallInfo.java new file mode 100644 index 00000000000..a4f5486ccf1 --- /dev/null +++ b/vespajlib/src/main/java/com/yahoo/nativec/MallInfo.java @@ -0,0 +1,30 @@ +package com.yahoo.nativec; + +import com.sun.jna.Structure; + +public class MallInfo { + private final static Throwable initException = NativeC.loadLibrary(MallInfo.class); + public static Throwable init() { + return initException; + } + + @Structure.FieldOrder({"arena", "ordblks", "smblks", "hblks", "hblkhd", "usmblks", "fsmblks", "uordblks", "fordblks", "keepcost"}) + public static class MallInfoStruct extends Structure { + public static class ByValue extends MallInfoStruct implements Structure.ByValue { } + public int arena; /* Non-mmapped space allocated (bytes) */ + public int ordblks; /* Number of free chunks */ + public int smblks; /* Number of free fastbin blocks */ + public int hblks; /* Number of mmapped regions */ + public int hblkhd; /* Space allocated in mmapped regions (bytes) */ + public int usmblks; /* See below */ + public int fsmblks; /* Space in freed fastbin blocks (bytes) */ + public int uordblks; /* Total allocated space (bytes) */ + public int fordblks; /* Total free space (bytes) */ + public int keepcost; /* Top-most, releasable space (bytes) */ + } + private static native MallInfoStruct.ByValue mallinfo(); + public MallInfo() { + mallinfo = mallinfo(); + } + private final MallInfoStruct mallinfo; +} diff --git a/vespajlib/src/main/java/com/yahoo/nativec/MallInfo2.java b/vespajlib/src/main/java/com/yahoo/nativec/MallInfo2.java new file mode 100644 index 00000000000..1ae3bc590e2 --- /dev/null +++ b/vespajlib/src/main/java/com/yahoo/nativec/MallInfo2.java @@ -0,0 +1,30 @@ +package com.yahoo.nativec; + +import com.sun.jna.Structure; + +public class MallInfo2 { + private final static Throwable initException = NativeC.loadLibrary(MallInfo2.class); + public static Throwable init() { + return initException; + } + + @Structure.FieldOrder({"arena", "ordblks", "smblks", "hblks", "hblkhd", "usmblks", "fsmblks", "uordblks", "fordblks", "keepcost"}) + public static class MallInfo2Struct extends Structure { + public static class ByValue extends MallInfo2Struct implements Structure.ByValue { } + public long arena; /* Non-mmapped space allocated (bytes) */ + public long ordblks; /* Number of free chunks */ + public long smblks; /* Number of free fastbin blocks */ + public long hblks; /* Number of mmapped regions */ + public long hblkhd; /* Space allocated in mmapped regions (bytes) */ + public long usmblks; /* See below */ + public long fsmblks; /* Space in freed fastbin blocks (bytes) */ + public long uordblks; /* Total allocated space (bytes) */ + public long fordblks; /* Total free space (bytes) */ + public long keepcost; /* Top-most, releasable space (bytes) */ + } + private static native MallInfo2Struct.ByValue mallinfo2(); + public MallInfo2() { + mallinfo = mallinfo2(); + } + private final MallInfo2Struct mallinfo; +} diff --git a/vespajlib/src/main/java/com/yahoo/nativec/NativeC.java b/vespajlib/src/main/java/com/yahoo/nativec/NativeC.java new file mode 100644 index 00000000000..4d808b4b155 --- /dev/null +++ b/vespajlib/src/main/java/com/yahoo/nativec/NativeC.java @@ -0,0 +1,20 @@ +package com.yahoo.nativec; + +import com.sun.jna.Native; +import com.sun.jna.Platform; + +class NativeC { + static Throwable loadLibrary(Class<?> cls) { + if (Platform.isLinux()) { + try { + Native.register(cls, Platform.C_LIBRARY_NAME); + } catch (Throwable e) { + return e; + } + } else { + return new RuntimeException("Platform is unsúpported. Only supported on linux."); + } + return null; + } + +} diff --git a/vespajlib/src/main/java/com/yahoo/nativec/PosixFAdvise.java b/vespajlib/src/main/java/com/yahoo/nativec/PosixFAdvise.java new file mode 100644 index 00000000000..3e2c26d2ef2 --- /dev/null +++ b/vespajlib/src/main/java/com/yahoo/nativec/PosixFAdvise.java @@ -0,0 +1,12 @@ +package com.yahoo.nativec; + +import com.sun.jna.LastErrorException; + +public class PosixFAdvise { + public static final int POSIX_FADV_DONTNEED = 4; // See /usr/include/linux/fadvise.h + private final static Throwable initException = NativeC.loadLibrary(PosixFAdvise.class); + public static Throwable init() { + return initException; + } + public static native int posix_fadvise(int fd, long offset, long len, int flag) throws LastErrorException; +} diff --git a/vespajlib/src/main/java/com/yahoo/nativec/package-info.java b/vespajlib/src/main/java/com/yahoo/nativec/package-info.java new file mode 100644 index 00000000000..cdf497ea39b --- /dev/null +++ b/vespajlib/src/main/java/com/yahoo/nativec/package-info.java @@ -0,0 +1,5 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +@ExportPackage +package com.yahoo.nativec; + +import com.yahoo.osgi.annotation.ExportPackage; diff --git a/vespajlib/src/test/java/com/yahoo/nativec/GlibCTestCase.java b/vespajlib/src/test/java/com/yahoo/nativec/GlibCTestCase.java new file mode 100644 index 00000000000..3a37116f606 --- /dev/null +++ b/vespajlib/src/test/java/com/yahoo/nativec/GlibCTestCase.java @@ -0,0 +1,33 @@ +package com.yahoo.nativec; + +import com.sun.jna.Platform; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class GlibCTestCase { + @Test + public void requireThatPosixFAdviseIsDetected() { + if (Platform.isLinux()) { + assertNull(PosixFAdvise.init()); + } else { + assertEquals("Platform is unsúpported. Only supported on linux.", PosixFAdvise.init().getMessage()); + } + } + + @Test + public void requireGlibcVersionIsDetected() { + if (Platform.isLinux()) { + assertNull(GLibcVersion.init()); + GLibcVersion version = new GLibcVersion(); + assertNotEquals("", version.version()); + assertTrue(version.major() >= 2); + assertTrue((version.major() >= 3) || ((version.major() == 2) && (version.minor() >= 17))); + } else { + assertEquals("Platform is unsúpported. Only supported on linux.", PosixFAdvise.init().getMessage()); + } + } +} diff --git a/vespajlib/src/test/java/com/yahoo/nativec/MallInfoTestCase.java b/vespajlib/src/test/java/com/yahoo/nativec/MallInfoTestCase.java new file mode 100644 index 00000000000..e6b3e1cbf85 --- /dev/null +++ b/vespajlib/src/test/java/com/yahoo/nativec/MallInfoTestCase.java @@ -0,0 +1,29 @@ +package com.yahoo.nativec; + +import com.sun.jna.Platform; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +public class MallInfoTestCase { + @Test + public void requireThatMallInfo2IsDetected() { + if (Platform.isLinux()) { + GLibcVersion version = new GLibcVersion(); + if ((version.major() >= 3) || ((version.major() == 2) && (version.minor() >= 33))) { + assertNull(MallInfo2.init()); + } + } else { + assertEquals("Platform is unsúpported. Only supported on linux.", MallInfo2.init().getMessage()); + } + } + @Test + public void requireThatMallInfoIsDetected() { + if (Platform.isLinux()) { + assertNull(MallInfo.init()); + } else { + assertEquals("Platform is unsúpported. Only supported on linux.", MallInfo.init().getMessage()); + } + } +} diff --git a/vespalib/CMakeLists.txt b/vespalib/CMakeLists.txt index c3c7fee2cef..7796ce7df28 100644 --- a/vespalib/CMakeLists.txt +++ b/vespalib/CMakeLists.txt @@ -12,8 +12,9 @@ vespa_define_module( APPS src/apps/make_fixture_macros src/apps/vespa-detect-hostname - src/apps/vespa-validate-hostname src/apps/vespa-drop-file-from-cache + src/apps/vespa-tsan-digest + src/apps/vespa-validate-hostname TESTS src/tests/alloc diff --git a/vespalib/src/apps/vespa-tsan-digest/.gitignore b/vespalib/src/apps/vespa-tsan-digest/.gitignore new file mode 100644 index 00000000000..ca770e0e9c3 --- /dev/null +++ b/vespalib/src/apps/vespa-tsan-digest/.gitignore @@ -0,0 +1 @@ +/vespa-tsan-digest diff --git a/vespalib/src/apps/vespa-tsan-digest/CMakeLists.txt b/vespalib/src/apps/vespa-tsan-digest/CMakeLists.txt new file mode 100644 index 00000000000..3214d833783 --- /dev/null +++ b/vespalib/src/apps/vespa-tsan-digest/CMakeLists.txt @@ -0,0 +1,9 @@ +# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(vespalib_vespa-tsan-digest_app + SOURCES + tsan_digest.cpp + OUTPUT_NAME vespa-tsan-digest + INSTALL bin + DEPENDS + vespalib +) diff --git a/vespalib/src/apps/vespa-tsan-digest/tsan_digest.cpp b/vespalib/src/apps/vespa-tsan-digest/tsan_digest.cpp new file mode 100644 index 00000000000..bebb32ac1ec --- /dev/null +++ b/vespalib/src/apps/vespa-tsan-digest/tsan_digest.cpp @@ -0,0 +1,278 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/vespalib/stllike/string.h> +#include <vespa/vespalib/util/stringfmt.h> +#include <vespa/vespalib/util/size_literals.h> +#include <xxhash.h> +#include <cassert> +#include <vector> +#include <map> +#include <memory> +#include <algorithm> +#include <unistd.h> +#include <string.h> + +using vespalib::make_string_short::fmt; + +constexpr auto npos = vespalib::string::npos; + +//----------------------------------------------------------------------------- + +size_t trace_limit = 7; + +//----------------------------------------------------------------------------- + +class Hasher { +private: + XXH3_state_t *_state; +public: + Hasher() : _state(XXH3_createState()) { assert(_state != nullptr && "Out of memory!"); } + ~Hasher() { XXH3_freeState(_state); } + void reset() { XXH3_64bits_reset(_state); } + void update(const char *buf, size_t len) { XXH3_64bits_update(_state, buf, len); } + uint64_t get() const { return XXH3_64bits_digest(_state); } +}; + +uint64_t get_hash(const std::vector<vespalib::string> &list) { + static Hasher hasher; + hasher.reset(); + for (const auto &item: list) { + hasher.update(item.data(), item.size()); + } + return hasher.get(); +} + +//----------------------------------------------------------------------------- + +class StackTrace { +private: + vespalib::string _heading; + std::vector<vespalib::string> _frames; + uint64_t _hash; +public: + StackTrace(const vespalib::string &heading) noexcept + : _heading(heading), _frames(), _hash() {} + void add_frame(const vespalib::string &frame) { + _frames.push_back(frame); + } + void done() { _hash = get_hash(_frames); } + uint64_t hash() const { return _hash; } + void dump(FILE *dst) const { + fprintf(dst, "%s\n", _heading.c_str()); + for (const auto &frame: _frames) { + fprintf(dst, "%s\n", frame.c_str()); + } + fprintf(dst, "\n"); + } +}; + +vespalib::string make_trace_heading(const vespalib::string &line) { + auto pos = line.find(" at 0x"); + if ((pos != npos) && (line.find("Location") == npos)) { + return line.substr(0, pos); + } + return line; +} + +std::vector<StackTrace> extract_traces(const std::vector<vespalib::string> &lines, size_t cutoff) { + std::vector<StackTrace> result; + for (size_t i = 1; (i < lines.size()) && (result.size() < cutoff); ++i) { + auto pos = lines[i].find("#0 "); + if (pos != npos) { + size_t start = i; + result.emplace_back(make_trace_heading(lines[i - 1])); + result.back().add_frame(lines[i]); + for (i = i + 1; i < lines.size(); ++i) { + if (((i - start) > trace_limit) || + (lines[i].find("#") == npos)) + { + break; + } + result.back().add_frame(lines[i]); + } + result.back().done(); + } + } + return result; +}; + +//----------------------------------------------------------------------------- + +enum class ReportType { UNKNOWN, RACE }; + +ReportType detect_report_type(const std::vector<vespalib::string> &lines) { + for (const auto &line: lines) { + if (starts_with(line, "WARNING: ThreadSanitizer: data race")) { + return ReportType::RACE; + } + } + return ReportType::UNKNOWN; +} + +//----------------------------------------------------------------------------- + +bool is_delimiter(const vespalib::string &line) { + // TSAN delimiter is 18=, look for at least 16= + return (line.find("================") < line.size()); +} + +void dump_delimiter(FILE *dst) { + fprintf(dst, "==================\n"); +} + +//----------------------------------------------------------------------------- + +struct Report { + using UP = std::unique_ptr<Report>; + virtual vespalib::string make_key() const = 0; + virtual void add(Report::UP report) = 0; + virtual size_t count() const = 0; + virtual void dump(FILE *dst) const = 0; + virtual ~Report() {} +}; + +class RawReport : public Report { +private: + std::vector<vespalib::string> _lines; +public: + RawReport(const std::vector<vespalib::string> &lines) + : _lines(lines) {} + vespalib::string make_key() const override { + return fmt("raw:%zu", get_hash(_lines)); + } + void add(Report::UP) override { + fprintf(stderr, "WARNING: hash collision for raw report\n"); + } + size_t count() const override { return 1; } + void dump(FILE *dst) const override { + for (const auto &line: _lines) { + fprintf(dst, "%s\n", line.c_str()); + } + } +}; + +class RaceReport : public Report { +private: + StackTrace _trace1; + StackTrace _trace2; + size_t _total; + size_t _inverted; + +public: + RaceReport(const StackTrace &trace1, const StackTrace &trace2) + : _trace1(trace1), _trace2(trace2), _total(1), _inverted(0) {} + + vespalib::string make_key() const override { + if (_trace2.hash() < _trace1.hash()) { + return fmt("race:%zu,%zu", _trace2.hash(), _trace1.hash()); + } + return fmt("race:%zu,%zu", _trace1.hash(), _trace2.hash()); + } + void add(Report::UP report) override { + // should have correct type due to key prefix + const RaceReport &rhs = dynamic_cast<RaceReport&>(*report); + ++_total; + if (_trace1.hash() != rhs._trace1.hash()) { + ++_inverted; + } + } + size_t count() const override { return _total; } + void dump(FILE *dst) const override { + fprintf(dst, "WARNING: ThreadSanitizer: data race\n"); + _trace1.dump(dst); + _trace2.dump(dst); + fprintf(dst, "INFO: total: %zu (inverted: %zu)\n", _total, _inverted); + } +}; + +//----------------------------------------------------------------------------- + +size_t total_reports = 0; +std::map<vespalib::string,Report::UP> reports; + +void handle_report(std::unique_ptr<Report> report) { + ++total_reports; + auto [pos, first] = reports.try_emplace(report->make_key(), std::move(report)); + if (!first) { + assert(report && "should still be valid"); + pos->second->add(std::move(report)); + } +} + +void make_report(const std::vector<vespalib::string> &lines) { + auto type = detect_report_type(lines); + if (type == ReportType::RACE) { + auto traces = extract_traces(lines, 2); + if (traces.size() == 2) { + return handle_report(std::make_unique<RaceReport>(traces[0], traces[1])); + } + } + return handle_report(std::make_unique<RawReport>(lines)); +} + +void handle_line(const vespalib::string &line) { + static bool inside = false; + static std::vector<vespalib::string> lines; + if (is_delimiter(line)) { + inside = !inside; + if (!inside && !lines.empty()) { + make_report(lines); + lines.clear(); + } + } else if (inside) { + lines.push_back(line); + } +} + +void read_input() { + char buf[64_Ki]; + bool eof = false; + vespalib::string line; + while (!eof) { + ssize_t res = read(STDIN_FILENO, buf, sizeof(buf)); + if (res < 0) { + throw fmt("error reading stdin: %s", strerror(errno)); + } + eof = (res == 0); + for (int i = 0; i < res; ++i) { + if (buf[i] == '\n') { + handle_line(line); + line.clear(); + } else { + line.push_back(buf[i]); + } + } + } + if (!line.empty()) { + handle_line(line); + } +} + +void write_output() { + std::vector<Report*> list; + list.reserve(reports.size()); + for (const auto &[key, value]: reports) { + list.push_back(value.get()); + } + std::sort(list.begin(), list.end(), + [](const auto &a, const auto &b) { + return (a->count() > b->count()); + }); + for (const auto *report: list) { + dump_delimiter(stdout); + report->dump(stdout); + dump_delimiter(stdout); + } + fprintf(stderr, "%zu reports in, %zu reports out\n", total_reports, reports.size()); +} + +int main(int, char **) { + try { + read_input(); + write_output(); + } catch (vespalib::string &err) { + fprintf(stderr, "%s\n", err.c_str()); + return 1; + } + return 0; +} diff --git a/yolean/abi-spec.json b/yolean/abi-spec.json index 45ba75d736d..6285cc54118 100644 --- a/yolean/abi-spec.json +++ b/yolean/abi-spec.json @@ -202,6 +202,7 @@ "methods": [ "public void <init>(com.yahoo.yolean.concurrent.ResourceFactory)", "public void <init>(java.util.function.Supplier)", + "public void preallocate(int)", "public final java.lang.Object alloc()", "public final void free(java.lang.Object)", "public java.util.Iterator iterator()" @@ -258,8 +259,9 @@ ], "methods": [ "public void <init>(com.yahoo.yolean.concurrent.ResourceFactory)", - "public final java.lang.Object alloc()", - "public final void free(java.lang.Object)", + "public void <init>(java.util.function.Supplier)", + "public java.lang.Object alloc()", + "public void free(java.lang.Object)", "public java.util.Iterator iterator()" ], "fields": [] diff --git a/yolean/src/main/java/com/yahoo/yolean/concurrent/ConcurrentResourcePool.java b/yolean/src/main/java/com/yahoo/yolean/concurrent/ConcurrentResourcePool.java index fae01e8ac34..bdd059f3e17 100644 --- a/yolean/src/main/java/com/yahoo/yolean/concurrent/ConcurrentResourcePool.java +++ b/yolean/src/main/java/com/yahoo/yolean/concurrent/ConcurrentResourcePool.java @@ -18,7 +18,9 @@ public class ConcurrentResourcePool<T> implements Iterable<T> { private final Queue<T> pool = new ConcurrentLinkedQueue<>(); private final Supplier<T> factory; - // TODO: Deprecate + /** @deprecated Use {@link ConcurrentResourcePool(Supplier)} instead */ + @Deprecated(forRemoval = true, since = "7") + @SuppressWarnings("removal") public ConcurrentResourcePool(ResourceFactory<T> factory) { this.factory = factory.asSupplier(); } @@ -27,6 +29,12 @@ public class ConcurrentResourcePool<T> implements Iterable<T> { this.factory = factory; } + public void preallocate(int instances) { + for (int i = 0; i < instances; i++) { + pool.offer(factory.get()); + } + } + /** * Allocates an instance of the resource to the requestor. * The resource will be allocated exclusively to the requestor until it calls free(instance). diff --git a/yolean/src/main/java/com/yahoo/yolean/concurrent/ResourceFactory.java b/yolean/src/main/java/com/yahoo/yolean/concurrent/ResourceFactory.java index 584647198a5..cc9acf69684 100644 --- a/yolean/src/main/java/com/yahoo/yolean/concurrent/ResourceFactory.java +++ b/yolean/src/main/java/com/yahoo/yolean/concurrent/ResourceFactory.java @@ -6,7 +6,7 @@ import java.util.function.Supplier; /** * @author baldersheim */ -// TODO: Deprecate +@Deprecated(forRemoval = true, since = "7") public abstract class ResourceFactory<T> { public abstract T create(); diff --git a/yolean/src/main/java/com/yahoo/yolean/concurrent/ResourcePool.java b/yolean/src/main/java/com/yahoo/yolean/concurrent/ResourcePool.java index 75f8c961349..895fa890beb 100644 --- a/yolean/src/main/java/com/yahoo/yolean/concurrent/ResourcePool.java +++ b/yolean/src/main/java/com/yahoo/yolean/concurrent/ResourcePool.java @@ -4,6 +4,7 @@ package com.yahoo.yolean.concurrent; import java.util.ArrayDeque; import java.util.Deque; import java.util.Iterator; +import java.util.function.Supplier; /** * <p>This implements a simple stack based resource pool. If you are out of resources new are allocated from the @@ -15,17 +16,24 @@ import java.util.Iterator; public final class ResourcePool<T> implements Iterable<T> { private final Deque<T> pool = new ArrayDeque<>(); - private final ResourceFactory<T> factory; + private final Supplier<T> factory; + /** @deprecated Use {@link ResourcePool( Supplier )} instead */ + @Deprecated(forRemoval = true, since = "7") + @SuppressWarnings("removal") public ResourcePool(ResourceFactory<T> factory) { + this(factory.asSupplier()); + } + + public ResourcePool(Supplier<T> factory) { this.factory = factory; } - public final T alloc() { - return pool.isEmpty() ? factory.create() : pool.pop(); + public T alloc() { + return pool.isEmpty() ? factory.get() : pool.pop(); } - public final void free(T e) { + public void free(T e) { pool.push(e); } diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java index 6f914a8e9a3..05294a5435b 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java @@ -9,6 +9,8 @@ import com.yahoo.vespa.curator.stats.ThreadLockStats; import org.apache.curator.framework.recipes.locks.InterProcessLock; import java.time.Duration; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.TimeUnit; /** @@ -21,6 +23,10 @@ import java.util.concurrent.TimeUnit; */ public class Lock implements Mutex { + private final Object monitor = new Object(); + private long nextSequenceNumber = 0; + private final Map<Long, Long> reentriesByThreadId = new HashMap<>(); + private final InterProcessLock mutex; private final String lockPath; @@ -52,14 +58,43 @@ public class Lock implements Mutex { throw new UncheckedTimeoutException("Timed out after waiting " + timeout + " to acquire lock '" + lockPath + "'"); } - threadLockStats.lockAcquired(); + + invoke(+1L, threadLockStats::lockAcquired); + } + + @FunctionalInterface + private interface TriConsumer { + void accept(String lockId, long reentryCountDiff, Map<Long, Long> reentriesByThreadId); + } + + // TODO(hakon): Remove once debugging is unnecessary + private void invoke(long reentryCountDiff, TriConsumer consumer) { + long threadId = Thread.currentThread().getId(); + final long sequenceNumber; + final Map<Long, Long> reentriesByThreadIdCopy; + synchronized (monitor) { + sequenceNumber = nextSequenceNumber++; + reentriesByThreadId.merge(threadId, reentryCountDiff, (oldValue, argumentValue) -> { + long sum = oldValue + argumentValue /* == reentryCountDiff */; + if (sum == 0) { + // Remove from map + return null; + } else { + return sum; + } + }); + reentriesByThreadIdCopy = Map.copyOf(reentriesByThreadId); + } + + String lockId = Integer.toHexString(System.identityHashCode(this)); + consumer.accept(lockId, sequenceNumber, reentriesByThreadIdCopy); } @Override public void close() { ThreadLockStats threadLockStats = LockStats.getForCurrentThread(); // Update metrics now before release() to avoid double-counting time in locked state. - threadLockStats.preRelease(); + invoke(-1L, threadLockStats::preRelease); try { mutex.release(); threadLockStats.postRelease(); @@ -72,6 +107,10 @@ public class Lock implements Mutex { protected String lockPath() { return lockPath; } + @Override + public String toString() { + return "Lock{" + lockPath + "}"; + } } diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/LockAttempt.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/LockAttempt.java index 887e2cd2700..1bbd3c7c734 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/LockAttempt.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/LockAttempt.java @@ -66,7 +66,7 @@ public class LockAttempt { public String getLockPath() { return lockPath; } public Instant getTimeAcquiredWasInvoked() { return callAcquireInstant; } public Duration getAcquireTimeout() { return timeout; } - public boolean getReentry() { return reentry; } + public boolean isReentry() { return reentry; } public LockState getLockState() { return lockState; } public Optional<Instant> getTimeLockWasAcquired() { return lockAcquiredInstant; } public boolean isAcquiring() { return lockAcquiredInstant.isEmpty(); } diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/LockStats.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/LockStats.java index e4f78a4f9e9..ecb344dedb9 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/LockStats.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/LockStats.java @@ -64,28 +64,33 @@ public class LockStats { } /** Must be invoked only after the first and non-reentry acquisition of the lock. */ - void notifyOfThreadHoldingLock(Thread currentThread, String lockPath) { + void notifyOfThreadHoldingLock(Thread currentThread, String lockPath, String lockId, + long sequenceNumber, Map<Long, Long> reentriesByThreadId) { Thread oldThread = lockPathsHeld.put(lockPath, currentThread); if (oldThread != null) { getLockMetrics(lockPath).incrementAcquireWithoutReleaseCount(); - logger.warning("Thread " + currentThread.getName() + - " reports it has the lock on " + lockPath + ", but thread " + oldThread.getName() + - " has not reported it released the lock"); + logger.warning("Thread " + currentThread.getName() + " reports it has the lock on " + + lockPath + ", but thread " + oldThread.getName() + + " has not reported it released the lock. " + lockId + "#" + sequenceNumber + + ", reentries by thread ID = " + reentriesByThreadId); } } /** Must be invoked only before the last and non-reentry release of the lock. */ - void notifyOfThreadReleasingLock(Thread currentThread, String lockPath) { + void notifyOfThreadReleasingLock(Thread currentThread, String lockPath, String lockId, + long sequenceNumber, Map<Long, Long> reentriesByThreadId) { Thread oldThread = lockPathsHeld.remove(lockPath); if (oldThread == null) { getLockMetrics(lockPath).incrementNakedReleaseCount(); - logger.warning("Thread " + currentThread.getName() + - " is releasing the lock " + lockPath + ", but nobody owns that lock"); + logger.warning("Thread " + currentThread.getName() + " is releasing the lock " + lockPath + + ", but nobody owns that lock. " + lockId + "#" + sequenceNumber + + ", reentries by thread ID = " + reentriesByThreadId); } else if (oldThread != currentThread) { getLockMetrics(lockPath).incrementForeignReleaseCount(); logger.warning("Thread " + currentThread.getName() + - " is releasing the lock " + lockPath + ", but it was owned by thread " - + oldThread.getName()); + " is releasing the lock " + lockPath + ", but it was owned by thread " + + oldThread.getName() + ". " + lockId + "#" + sequenceNumber + + ", reentries by thread ID = " + reentriesByThreadId); } } diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/ThreadLockStats.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/ThreadLockStats.java index d4511bd04fb..7f8eafdcf7f 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/ThreadLockStats.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/ThreadLockStats.java @@ -6,6 +6,7 @@ import com.yahoo.vespa.curator.Lock; import java.time.Duration; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentLinkedDeque; @@ -97,7 +98,7 @@ public class ThreadLockStats { } /** Mutable method (see class doc) */ - public void lockAcquired() { + public void lockAcquired(String lockId, long sequenceNumber, Map<Long, Long> reentriesByThreadId) { withLastLockAttempt(lockAttempt -> { // Note on the order of lockAcquired() vs notifyOfThreadHoldingLock(): When the latter is // invoked, other threads may query e.g. isAcquired() on the lockAttempt, which would @@ -105,19 +106,21 @@ public class ThreadLockStats { // but seems better to ensure LockAttempt is updated first. lockAttempt.lockAcquired(); - if (!lockAttempt.getReentry()) { - LockStats.getGlobal().notifyOfThreadHoldingLock(thread, lockAttempt.getLockPath()); + if (!lockAttempt.isReentry()) { + LockStats.getGlobal().notifyOfThreadHoldingLock(thread, lockAttempt.getLockPath(), + lockId, sequenceNumber, reentriesByThreadId); } }); } /** Mutable method (see class doc) */ - public void preRelease() { + public void preRelease(String lockId, long sequenceNumber, Map<Long, Long> reentriesByThreadId) { withLastLockAttempt(lockAttempt -> { // Note on the order of these two statement: Same concerns apply here as in lockAcquired(). - if (!lockAttempt.getReentry()) { - LockStats.getGlobal().notifyOfThreadReleasingLock(thread, lockAttempt.getLockPath()); + if (!lockAttempt.isReentry()) { + LockStats.getGlobal().notifyOfThreadReleasingLock(thread, lockAttempt.getLockPath(), + lockId, sequenceNumber, reentriesByThreadId); } lockAttempt.preRelease(); |