diff options
56 files changed, 1024 insertions, 553 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3a86f99d17c..9e18ffbf487 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,5 +1,7 @@ -<!-- Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> +<!-- Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> + # Contributing to Vespa + Contributions to [Vespa](http://github.com/vespa-engine/vespa), [Vespa system tests](http://github.com/vespa-engine/system-test), [Vespa samples](https://github.com/vespa-engine/sample-apps) @@ -7,58 +9,74 @@ and the [Vespa documentation](http://github.com/vespa-engine/documentation) are This documents tells you what you need to know to contribute. ## Open development + All work on Vespa happens directly on Github, using the [Github flow model](https://guides.github.com/introduction/flow/). -We release the master branch a few times a week and you should expect it to almost always work. -In addition to the [public Screwdriver build](https://cd.screwdriver.cd/pipelines/6386) -we have a large acceptance and performance test suite which -is also run continuously. We plan to add this to the open source code base later. +We release the master branch four times a week and you should expect it to always work. +The continuous build of Vespa is at [https://factory.vespa.oath.cloud](https://factory.vespa.oath.cloud). +You can follow the fate of each commit there. -All pull requests are reviewed by a member of the Vespa Committers team. +All pull requests must be approved by a +[Vespa Committer](https://github.com/orgs/vespa-engine/people). You can find a suitable reviewer in the OWNERS file upward in the source tree from -where you are making the change (the OWNERS have a special responsibility for +where you are making the change (OWNERS have a special responsibility for ensuring the long-term integrity of a portion of the code). -If you want to become a committer/OWNER making some quality contributions is the way to start. +The way to become a committer (and OWNER) is to make some quality contributions +to an area of the code. See [GOVERNANCE](GOVERNANCE.md) for more details. -***Creating a Pull Request*** +### Creating a Pull Request -Please follow [best practices](https://github.com/trein/dev-best-practices/wiki/Git-Commit-Best-Practices) for creating git commits. +Please follow +[best practices](https://github.com/trein/dev-best-practices/wiki/Git-Commit-Best-Practices) +for creating git commits. -When your code is ready to be submitted, [submit a pull request](https://help.github.com/articles/creating-a-pull-request/) to begin the code review process. +When your code is ready to be submitted, +[submit a pull request](https://help.github.com/articles/creating-a-pull-request/) +to request a code review. -We only seek to accept code that you are authorized to contribute to the project. We have added a pull request template on our projects so that your contributions are made with the following confirmation: +We only seek to accept code that you are authorized to contribute to the project. +We have added a pull request template on our projects so that your contributions are made +with the following confirmation: > I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner. ## Versioning + Vespa uses semantic versioning - see [vespa versions](http://docs.vespa.ai/en/vespa-versions.html). Notice in particular that any Java API in a package having a @PublicAPI -annotation in the package-info file cannot be changed in an incompatible way -between major versions: Existing types and method signatures must be preserved +annotation in the package-info, and no @Beta annotation on the class, +cannot be changed in an incompatible way between major versions: +Existing types and method signatures must be preserved (but can be marked deprecated). +We verify ABI compatibility during the regular Java build you'll run with Maven (mvn install). +This build step will also fail if you *add* to public API's, which is fine if there's a good reason +to do it. In that case update the ABI spec as instructed in the error message. + ## Issues + We track issues in [GitHub issues](https://github.com/vespa-engine/vespa/issues). It is fine to submit issues also for feature requests and ideas, whether or not you intend to work on them. -There is also a [ToDo list](TODO.md) for larger things which nobody are working on yet. +There is also a [ToDo list](TODO.md) for larger things nobody are working on yet. ## Community -If you have questions, want to share your experience or help others, please join our community on [Stack Overflow](http://stackoverflow.com/questions/tagged/vespa). + +If you have questions, want to share your experience or help others, +join our [Slack channel](http://slack.vespa.ai). +See also [Stack Overflow questions tagged Vespa](http://stackoverflow.com/questions/tagged/vespa), +and feel free to add your own. ### Getting started -See [README](README.md) for how to build and test Vespa. -For an overview of the modules, see [Code-map.md](Code-map.md). +See [README](README.md) for how to build and test Vespa. +[Code-map.md](Code-map.md) provides an overview of the modules of Vespa. More details are in the READMEs of each module. ## License and copyright + If you add new files you are welcome to use your own copyright. In any case the code (or documentation) you submit will be licensed under the Apache 2.0 license. - -## Code of Conduct - -We encourage inclusive and professional interactions on our project. We welcome everyone to open an issue, improve the documentation, report bug or submit a pull request. By participating in this project, you agree to abide by the [Verizon Media Code of Conduct](Code-of-Conduct.md). If you feel there is a conduct issue related to this project, please raise it per the Code of Conduct process and we will address it. diff --git a/GOVERNANCE.md b/GOVERNANCE.md new file mode 100644 index 00000000000..617f2d3c27c --- /dev/null +++ b/GOVERNANCE.md @@ -0,0 +1,58 @@ +<!-- Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> + +# Vespa Governance + +Anyone is welcome to contribute to Vespa, by [creating pull requests](CONTRIBUTING.md) +or taking part in discussions about features, e.g in [GitHub issues](https://github.com/vespa-engine/vespa/issues). + +This document defines who has authority to do what on the Vespa open source project. + +While a formal assignment of authority is necessary as a fallback mechanism, it is +not the way decisions regarding Vespa are usually made, or expected to be made. +The community sees disagreement as good as a diversity of perspectives having influence +leads to better decisions. We seek to resolve disagreement by reaching +consensus based on the merit of technical arguments, with no regards to status or +formal role. This almost always works. + +# Vespa roles + +There are two formal roles in Vespa: + +* __Vespa Committers__ have the right to merge pull requests into the Vespa repo. +* __The Vespa Board__ has final authority over all decisions made on the project. + +# Vespa committers + +A __Vespa committer__ is able to merge pull requests into the Vespa repositories under +[https://github.com/vespa-engine](https://github.com/vespa-engine), including into +the master branch which is released to production. + +To become a Vespa Committer, start making PR's that are approved. When a sufficient +trust is built through working on Vespa this way, any current Vespa Committer can nominate +you to the Vespa board for the committer role. + +Every pull requests must be approved by a Vespa Committer which is different from the +author of the request, either by clicking "approve", by making a comment indicating agreement, +or simply merging it. As a rule, approval should happen before merge, but this is not strictly +required as the approver can revert. + +The Vespa committers are listed here: +[https://github.com/orgs/vespa-engine/people](https://github.com/orgs/vespa-engine/people) + +# The Vespa board + +The __Vespa board__ has final authority over all decisions regarding the Vespa project, +including + +* whether to merge (or keep) a PR in the event of any dispute, +* who should be committers and who should be members of the board, and +* whether to make any changes to the governance policy (this document). + +The board consists of 4 people, and makes decisions by majority vote. +On a tie the leader of the board has a double vote. + +The board members are: +* Jon Bratseth (bratseth) - leader +* Kristian Aune (kkraune) +* Kim Johansen (johans1) +* Frode Lundgren (frodelu) diff --git a/README.md b/README.md index 7d3be172cb5..5c0de9f8202 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ The open big data serving engine - Store, search, organize and make machine-lear over big data at serving time. This is the primary repository for Vespa where all development is happening. -New production releases from this repository's master branch are made each weekday from Monday to Thursday. +New production releases from this repository's master branch are made each weekday from Monday through Thursday. * Home page: [https://vespa.ai](https://vespa.ai) * Documentation: [https://docs.vespa.ai](https://docs.vespa.ai) @@ -50,7 +50,7 @@ Or deploy your Vespa applications to the cloud service: [https://cloud.vespa.ai] - Explore the [sample applications](https://github.com/vespa-engine/sample-apps/tree/master) - Follow the [Vespa Blog](https://blog.vespa.ai/) for feature updates / use cases -Full documentation is available on [https://docs.vespa.ai](https://docs.vespa.ai). +Full documentation is at [https://docs.vespa.ai](https://docs.vespa.ai). ## Contribute 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 694193923c5..528b41efb6a 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 @@ -83,6 +83,7 @@ public interface ModelContext { @ModelFeatureFlag(owners = {"baldersheim"}) default int largeRankExpressionLimit() { return 8192; } @ModelFeatureFlag(owners = {"baldersheim"}) default int maxConcurrentMergesPerNode() { throw new UnsupportedOperationException("TODO specify default value"); } @ModelFeatureFlag(owners = {"baldersheim"}) default int maxMergeQueueSize() { throw new UnsupportedOperationException("TODO specify default value"); } + @ModelFeatureFlag(owners = {"vekterli", "geirst"}) default boolean ignoreMergeQueueLimit() { throw new UnsupportedOperationException("TODO specify default value"); } @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"}) default boolean enableFeedBlockInDistributor() { return true; } diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java index 570638e7e93..7c3d8e4763f 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java @@ -57,6 +57,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea private int numDistributorStripes = 0; private int maxConcurrentMergesPerNode = 16; private int maxMergeQueueSize = 1024; + private boolean ignoreMergeQueueLimit = false; private int largeRankExpressionLimit = 8192; private boolean allowDisableMtls = true; private List<X509Certificate> operatorCertificates = Collections.emptyList(); @@ -104,6 +105,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea @Override public int largeRankExpressionLimit() { return largeRankExpressionLimit; } @Override public int maxConcurrentMergesPerNode() { return maxConcurrentMergesPerNode; } @Override public int maxMergeQueueSize() { return maxMergeQueueSize; } + @Override public boolean ignoreMergeQueueLimit() { return ignoreMergeQueueLimit; } @Override public double resourceLimitDisk() { return resourceLimitDisk; } @Override public double resourceLimitMemory() { return resourceLimitMemory; } @Override public double minNodeRatioPerGroup() { return minNodeRatioPerGroup; } @@ -165,6 +167,11 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea return this; } + public TestProperties setIgnoreMergeQueueLimit(boolean ignoreMergeQueueLimit) { + this.ignoreMergeQueueLimit = ignoreMergeQueueLimit; + return this; + } + public TestProperties setDefaultTermwiseLimit(double limit) { defaultTermwiseLimit = limit; return this; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorServerProducer.java b/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorServerProducer.java index e89d45e8b83..7acae9b32f9 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorServerProducer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/storagecluster/StorServerProducer.java @@ -32,6 +32,7 @@ public class StorServerProducer implements StorServerConfig.Producer { private Integer maxMergesPerNode; private Integer queueSize; private Integer bucketDBStripeBits; + private Boolean ignoreMergeQueueLimit; private StorServerProducer setMaxMergesPerNode(Integer value) { if (value != null) { @@ -54,6 +55,7 @@ public class StorServerProducer implements StorServerConfig.Producer { this.clusterName = clusterName; maxMergesPerNode = featureFlags.maxConcurrentMergesPerNode(); queueSize = featureFlags.maxMergeQueueSize(); + ignoreMergeQueueLimit = featureFlags.ignoreMergeQueueLimit(); } @Override @@ -73,5 +75,8 @@ public class StorServerProducer implements StorServerConfig.Producer { if (bucketDBStripeBits != null) { builder.content_node_bucket_db_stripe_bits(bucketDBStripeBits); } + if (ignoreMergeQueueLimit != null) { + builder.disable_queue_limits_for_chained_merges(ignoreMergeQueueLimit); + } } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/StorageClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/StorageClusterTest.java index 9a681003293..fd85ea3d024 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/StorageClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/StorageClusterTest.java @@ -118,6 +118,7 @@ public class StorageClusterTest { StorServerConfig config = new StorServerConfig(builder); assertEquals(16, config.max_merges_per_node()); assertEquals(1024, config.max_merge_queue_size()); + assertFalse(config.disable_queue_limits_for_chained_merges()); } @Test @@ -134,17 +135,30 @@ public class StorageClusterTest { assertEquals(1024, config.max_merges_per_node()); assertEquals(1024*10, config.max_merge_queue_size()); } - @Test - public void testMergeFeatureFlags() { + + private StorServerConfig configFromProperties(TestProperties properties) { StorServerConfig.Builder builder = new StorServerConfig.Builder(); - parse(cluster("foofighters", ""), new TestProperties().setMaxMergeQueueSize(1919).setMaxConcurrentMergesPerNode(37)).getConfig(builder); + parse(cluster("foofighters", ""), properties).getConfig(builder); + return new StorServerConfig(builder); + } - StorServerConfig config = new StorServerConfig(builder); + @Test + public void testMergeFeatureFlags() { + var config = configFromProperties(new TestProperties().setMaxMergeQueueSize(1919).setMaxConcurrentMergesPerNode(37)); assertEquals(37, config.max_merges_per_node()); assertEquals(1919, config.max_merge_queue_size()); } @Test + public void ignore_merge_queue_limit_can_be_controlled_by_feature_flag() { + var config = configFromProperties(new TestProperties().setIgnoreMergeQueueLimit(true)); + assertTrue(config.disable_queue_limits_for_chained_merges()); + + config = configFromProperties(new TestProperties().setIgnoreMergeQueueLimit(false)); + assertFalse(config.disable_queue_limits_for_chained_merges()); + } + + @Test public void testVisitors() { StorVisitorConfig.Builder builder = new StorVisitorConfig.Builder(); parse(cluster("bees", diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java index 71f1571b9c8..2e8685887c6 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java @@ -1,4 +1,4 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.proxy; import com.yahoo.concurrent.DaemonThreadFactory; @@ -11,6 +11,7 @@ import com.yahoo.jrt.Supervisor; import com.yahoo.jrt.Target; import com.yahoo.jrt.Transport; import com.yahoo.vespa.config.ConfigCacheKey; +import com.yahoo.vespa.config.ConfigKey; import com.yahoo.vespa.config.RawConfig; import com.yahoo.vespa.config.TimingValues; import com.yahoo.vespa.config.protocol.JRTServerConfigRequest; @@ -37,8 +38,8 @@ import static java.util.concurrent.TimeUnit.SECONDS; */ class RpcConfigSourceClient implements ConfigSourceClient, Runnable { - private final static Logger log = Logger.getLogger(RpcConfigSourceClient.class.getName()); - private static final double timingValuesRatio = 0.8; + private static final Logger log = Logger.getLogger(RpcConfigSourceClient.class.getName()); + private static final TimingValues timingValues = createTimingValues(); private final Supervisor supervisor = new Supervisor(new Transport("config-source-client")); @@ -47,7 +48,6 @@ class RpcConfigSourceClient implements ConfigSourceClient, Runnable { private final Map<ConfigCacheKey, Subscriber> activeSubscribers = new ConcurrentHashMap<>(); private final MemoryCache memoryCache; private final DelayedResponses delayedResponses; - private final static TimingValues timingValues; private final ScheduledExecutorService nextConfigScheduler = Executors.newScheduledThreadPool(1, new DaemonThreadFactory("next config")); private final ScheduledFuture<?> nextConfigFuture; @@ -57,16 +57,6 @@ class RpcConfigSourceClient implements ConfigSourceClient, Runnable { Executors.newScheduledThreadPool(1, new DaemonThreadFactory("delayed responses")); private final ScheduledFuture<?> delayedResponsesFuture; - static { - // Proxy should time out before clients upon subscription. - TimingValues tv = new TimingValues(); - tv.setUnconfiguredDelay((long)(tv.getUnconfiguredDelay()* timingValuesRatio)). - setConfiguredErrorDelay((long)(tv.getConfiguredErrorDelay()* timingValuesRatio)). - setSubscribeTimeout((long)(tv.getSubscribeTimeout()* timingValuesRatio)). - setConfiguredErrorTimeout(-1); // Never cache errors - timingValues = tv; - } - RpcConfigSourceClient(RpcServer rpcServer, ConfigSourceSet configSourceSet, MemoryCache memoryCache) { this.rpcServer = rpcServer; this.configSourceSet = configSourceSet; @@ -74,35 +64,31 @@ class RpcConfigSourceClient implements ConfigSourceClient, Runnable { this.delayedResponses = new DelayedResponses(); checkConfigSources(); nextConfigFuture = nextConfigScheduler.scheduleAtFixedRate(this, 0, 10, MILLISECONDS); - requester = JRTConfigRequester.create(configSourceSet, timingValues); + this.requester = JRTConfigRequester.create(configSourceSet, timingValues); DelayedResponseHandler command = new DelayedResponseHandler(delayedResponses, memoryCache, rpcServer); - delayedResponsesFuture = delayedResponsesScheduler.scheduleAtFixedRate(command, 5, 1, SECONDS); + this.delayedResponsesFuture = delayedResponsesScheduler.scheduleAtFixedRate(command, 5, 1, SECONDS); } /** * Checks if config sources are available */ private void checkConfigSources() { - if (configSourceSet == null || configSourceSet.getSources() == null || configSourceSet.getSources().size() == 0) { - log.log(Level.WARNING, "No config sources defined, could not check connection"); - } else { - Request req = new Request("ping"); - for (String configSource : configSourceSet.getSources()) { - Spec spec = new Spec(configSource); - Target target = supervisor.connect(spec); - target.invokeSync(req, 30.0); - if (target.isValid()) { - log.log(Level.FINE, () -> "Created connection to config source at " + spec.toString()); - return; - } else { - log.log(Level.INFO, "Could not connect to config source at " + spec.toString()); - } - target.close(); - } - String extra = ""; - log.log(Level.INFO, "Could not connect to any config source in set " + configSourceSet.toString() + - ", please make sure config server(s) are running. " + extra); + if (configSourceSet == null || configSourceSet.getSources() == null || configSourceSet.getSources().size() == 0) + throw new IllegalArgumentException("No config sources defined, could not check connection"); + + Request req = new Request("ping"); + for (String configSource : configSourceSet.getSources()) { + Spec spec = new Spec(configSource); + Target target = supervisor.connect(spec); + target.invokeSync(req, 30.0); + if (target.isValid()) + return; + + log.log(Level.INFO, "Could not connect to config source at " + spec.toString()); + target.close(); } + log.log(Level.INFO, "Could not connect to any config source in set " + configSourceSet.toString() + + ", please make sure config server(s) are running."); } /** @@ -126,7 +112,7 @@ class RpcConfigSourceClient implements ConfigSourceClient, Runnable { DelayedResponse delayedResponse = new DelayedResponse(request); delayedResponses.add(delayedResponse); - final ConfigCacheKey configCacheKey = new ConfigCacheKey(input.getKey(), input.getDefMd5()); + ConfigCacheKey configCacheKey = new ConfigCacheKey(input.getKey(), input.getDefMd5()); RawConfig cachedConfig = memoryCache.get(configCacheKey); boolean needToGetConfig = true; @@ -219,40 +205,41 @@ class RpcConfigSourceClient implements ConfigSourceClient, Runnable { } /** - * This method will be called when a response with changed config is received from upstream - * (content or generation has changed) or the server timeout has elapsed. + * Updates subscribers with new config. This method will be called when a response with changed config is + * received from upstream (content or generation has changed) or the server timeout has elapsed. * * @param config new config */ public void updateSubscribers(RawConfig config) { - log.log(Level.FINE, () -> "Config updated for " + config.getKey() + "," + config.getGeneration()); + ConfigKey<?> key = config.getKey(); + long generation = config.getGeneration(); + log.log(Level.FINE, () -> "Config updated for " + key + "," + generation); DelayQueue<DelayedResponse> responseDelayQueue = delayedResponses.responses(); + if (responseDelayQueue.size() == 0) return; + + log.log(Level.FINE, () -> "Delayed response queue has " + responseDelayQueue.size() + " elements"); log.log(Level.FINEST, () -> "Delayed response queue: " + responseDelayQueue); - if (responseDelayQueue.size() == 0) { - log.log(Level.FINE, () -> "There exists no matching element on delayed response queue for " + config.getKey()); - return; - } else { - log.log(Level.FINE, () -> "Delayed response queue has " + responseDelayQueue.size() + " elements"); - } boolean found = false; for (DelayedResponse response : responseDelayQueue.toArray(new DelayedResponse[0])) { JRTServerConfigRequest request = response.getRequest(); - if (request.getConfigKey().equals(config.getKey()) + if (request.getConfigKey().equals(key) // Generation 0 is special, used when returning empty sentinel config - && (config.getGeneration() >= request.getRequestGeneration() || config.getGeneration() == 0)) { + && (generation >= request.getRequestGeneration() || generation == 0)) { if (delayedResponses.remove(response)) { found = true; - log.log(Level.FINE, () -> "Call returnOkResponse for " + config.getKey() + "," + config.getGeneration()); + log.log(Level.FINE, () -> "Call returnOkResponse for " + key + "," + generation); + if (config.getPayload().getData().getByteLength() == 0) + log.log(Level.WARNING, () -> "Call returnOkResponse for " + key + "," + generation + " with empty config"); rpcServer.returnOkResponse(request, config); } else { - log.log(Level.INFO, "Could not remove " + config.getKey() + " from delayedResponses queue, already removed"); + log.log(Level.INFO, "Could not remove " + key + " from delayedResponses queue, already removed"); } } } if (!found) { - log.log(Level.FINE, () -> "Found no recipient for " + config.getKey() + " in delayed response queue"); + log.log(Level.FINE, () -> "Found no recipient for " + key + " in delayed response queue"); } - log.log(Level.FINE, () -> "Finished updating config for " + config.getKey() + "," + config.getGeneration()); + log.log(Level.FINE, () -> "Finished updating config for " + key + "," + generation); } @Override @@ -268,4 +255,14 @@ class RpcConfigSourceClient implements ConfigSourceClient, Runnable { updateSubscribers(newConfig); } + private static TimingValues createTimingValues() { + // Proxy should time out before clients upon subscription. + double timingValuesRatio = 0.8; + TimingValues tv = new TimingValues(); + tv.setFixedDelay((long) (tv.getFixedDelay() * timingValuesRatio)). + setSubscribeTimeout((long) (tv.getSubscribeTimeout() * timingValuesRatio)). + setConfiguredErrorTimeout(-1); // Never cache errors + return tv; + } + } diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java index 853bc4dfc00..7713d509f69 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java @@ -1,4 +1,4 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.subscription.impl; import com.yahoo.config.ConfigInstance; @@ -9,7 +9,6 @@ import com.yahoo.jrt.RequestWaiter; import com.yahoo.vespa.config.Connection; import com.yahoo.vespa.config.ConnectionPool; import com.yahoo.vespa.config.ErrorCode; -import com.yahoo.vespa.config.ErrorType; import com.yahoo.vespa.config.TimingValues; import com.yahoo.vespa.config.protocol.JRTClientConfigRequest; import com.yahoo.vespa.config.protocol.JRTConfigRequestFactory; @@ -43,17 +42,20 @@ public class JRTConfigRequester implements RequestWaiter { public static final ConfigSourceSet defaultSourceSet = ConfigSourceSet.createDefault(); private static final JRTManagedConnectionPools managedPool = new JRTManagedConnectionPools(); private static final int TRACELEVEL = 6; - private final TimingValues timingValues; - private boolean fatalFailures = false; - private final ScheduledThreadPoolExecutor scheduler; - private Instant noApplicationWarningLogged = Instant.MIN; private static final Duration delayBetweenWarnings = Duration.ofSeconds(60); - private final ConnectionPool connectionPool; - private final ConfigSourceSet configSourceSet; static final float randomFraction = 0.2f; /* Time to be added to server timeout to create client timeout. This is the time allowed for the server to respond after serverTimeout has elapsed. */ private static final Double additionalTimeForClientTimeout = 10.0; + private final TimingValues timingValues; + private final ScheduledThreadPoolExecutor scheduler; + + private final ConnectionPool connectionPool; + private final ConfigSourceSet configSourceSet; + + private Instant noApplicationWarningLogged = Instant.MIN; + private int failures = 0; + /** * Returns a new requester * @@ -129,15 +131,13 @@ public class JRTConfigRequester implements RequestWaiter { Trace trace = jrtReq.getResponseTrace(); trace.trace(TRACELEVEL, "JRTConfigRequester.doHandle()"); log.log(FINEST, () -> trace.toString()); - if (validResponse) { + if (validResponse) handleOKRequest(jrtReq, sub); - } else { - logWhenErrorResponse(jrtReq, connection); + else handleFailedRequest(jrtReq, sub, connection); - } } - private void logWhenErrorResponse(JRTClientConfigRequest jrtReq, Connection connection) { + private void logError(JRTClientConfigRequest jrtReq, Connection connection) { switch (jrtReq.errorCode()) { case com.yahoo.jrt.ErrorCode.CONNECTION: log.log(FINE, () -> "Request callback failed: " + jrtReq.errorMessage() + @@ -160,77 +160,36 @@ public class JRTConfigRequester implements RequestWaiter { } private void handleFailedRequest(JRTClientConfigRequest jrtReq, JRTConfigSubscription<ConfigInstance> sub, Connection connection) { - final boolean configured = (sub.getConfigState().getConfig() != null); - if (configured) { - // The subscription object has an "old" config, which is all we have to offer back now - log.log(INFO, "Failure of config subscription, clients will keep existing config until resolved: " + sub); - } - ErrorType errorType = ErrorType.getErrorType(jrtReq.errorCode()); + logError(jrtReq, connection); + + // The subscription object has an "old" config, which is all we have to offer back now + log.log(INFO, "Failure of config subscription tp " + connection.getAddress() + + ", clients will keep existing config until resolved: " + sub); connectionPool.setError(connection, jrtReq.errorCode()); - long delay = calculateFailedRequestDelay(errorType, fatalFailures, timingValues, configured); - if (errorType == ErrorType.TRANSIENT) { - handleTransientlyFailed(jrtReq, sub, delay, connection); - } else { - handleFatallyFailed(jrtReq, sub, delay); - } + failures++; + long delay = calculateFailedRequestDelay(failures, timingValues); + // The logging depends on whether we are configured or not. + Level logLevel = sub.getConfigState().getConfig() == null ? Level.FINE : Level.INFO; + log.log(logLevel, () -> "Request for config " + jrtReq.getShortDescription() + "' failed with error code " + + jrtReq.errorCode() + " (" + jrtReq.errorMessage() + "), scheduling new request " + + " in " + delay + " ms"); + scheduleNextRequest(jrtReq, sub, delay, calculateErrorTimeout()); } - static long calculateFailedRequestDelay(ErrorType errorType, - boolean fatalFailures, - TimingValues timingValues, - boolean configured) { - long delay = configured ? timingValues.getConfiguredErrorDelay() : timingValues.getUnconfiguredDelay(); + static long calculateFailedRequestDelay(int failures, TimingValues timingValues) { + long delay = timingValues.getFixedDelay() * (long)Math.pow(2, failures); + delay = Math.min(60_000, delay); + delay = timingValues.getPlusMinusFractionRandom(delay, randomFraction); - switch (errorType) { - case TRANSIENT: - delay = timingValues.getRandomTransientDelay(delay); - break; - case FATAL: - delay = timingValues.getFixedDelay() + (fatalFailures ? delay : 0); - delay = timingValues.getPlusMinusFractionRandom(delay, randomFraction); - break; - default: - throw new IllegalArgumentException("Unknown error type " + errorType); - } return delay; } - private void handleTransientlyFailed(JRTClientConfigRequest jrtReq, - JRTConfigSubscription<ConfigInstance> sub, - long delay, - Connection connection) { - fatalFailures = false; - log.log(INFO, "Connection to " + connection.getAddress() + - " failed or timed out, clients will keep existing config, will keep trying."); - scheduleNextRequest(jrtReq, sub, delay, calculateErrorTimeout()); - } - private long calculateErrorTimeout() { return timingValues.getPlusMinusFractionRandom(timingValues.getErrorTimeout(), randomFraction); } - /** - * This handles a fatal error both in the case that the subscriber is configured and not. - * The difference is in the delay (passed from outside) and the log level used for - * error message. - * - * @param jrtReq a JRT config request - * @param sub a config subscription - * @param delay delay before sending a new request - */ - private void handleFatallyFailed(JRTClientConfigRequest jrtReq, JRTConfigSubscription<ConfigInstance> sub, long delay) { - fatalFailures = true; - // The logging depends on whether we are configured or not. - Level logLevel = sub.getConfigState().getConfig() == null ? Level.FINE : Level.INFO; - String logMessage = "Request for config " + jrtReq.getShortDescription() + "' failed with error code " + - jrtReq.errorCode() + " (" + jrtReq.errorMessage() + "), scheduling new connect " + - " in " + delay + " ms"; - log.log(logLevel, logMessage); - scheduleNextRequest(jrtReq, sub, delay, calculateErrorTimeout()); - } - private void handleOKRequest(JRTClientConfigRequest jrtReq, JRTConfigSubscription<ConfigInstance> sub) { - fatalFailures = false; + failures = 0; noApplicationWarningLogged = Instant.MIN; sub.setLastCallBackOKTS(Instant.now()); log.log(FINE, () -> "OK response received in handleOkRequest: " + jrtReq); @@ -303,9 +262,7 @@ public class JRTConfigRequester implements RequestWaiter { } } - boolean getFatalFailures() { - return fatalFailures; - } + int getFailures() { return failures; } // TODO: Should be package private, used in integrationtest.rb in system tests public ConnectionPool getConnectionPool() { diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java index e83fc7aefc5..d8551c37f41 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java @@ -15,6 +15,7 @@ import com.yahoo.vespa.config.protocol.Payload; import java.time.Duration; import java.time.Instant; +import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; @@ -39,7 +40,7 @@ public class JRTConfigSubscription<T extends ConfigInstance> extends ConfigSubsc * The queue containing either nothing or the one (newest) request that has got callback from JRT, * but has not yet been handled. */ - private LinkedBlockingQueue<JRTClientConfigRequest> reqQueue = new LinkedBlockingQueue<>(); + private BlockingQueue<JRTClientConfigRequest> reqQueue = new LinkedBlockingQueue<>(); private ConfigSourceSet sources; public JRTConfigSubscription(ConfigKey<T> key, ConfigSubscriber subscriber, ConfigSource source, TimingValues timingValues) { @@ -134,9 +135,7 @@ public class JRTConfigSubscription<T extends ConfigInstance> extends ConfigSubsc return configInstance; } - LinkedBlockingQueue<JRTClientConfigRequest> getReqQueue() { - return reqQueue; - } + BlockingQueue<JRTClientConfigRequest> getReqQueue() { return reqQueue; } @Override public boolean subscribe(long timeout) { diff --git a/config/src/main/java/com/yahoo/vespa/config/TimingValues.java b/config/src/main/java/com/yahoo/vespa/config/TimingValues.java index 235928a7d0b..56f85845aa0 100644 --- a/config/src/main/java/com/yahoo/vespa/config/TimingValues.java +++ b/config/src/main/java/com/yahoo/vespa/config/TimingValues.java @@ -1,4 +1,4 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config; import java.util.Random; @@ -19,8 +19,6 @@ public class TimingValues { private long configuredErrorTimeout = -1; // Don't ever timeout (and do not use error response) when we are already configured private long fixedDelay = 5000; - private long unconfiguredDelay = 1000; - private long configuredErrorDelay = 15000; private final Random rand; public TimingValues() { @@ -35,15 +33,11 @@ public class TimingValues { long errorTimeout, long initialTimeout, long subscribeTimeout, - long unconfiguredDelay, - long configuredErrorDelay, long fixedDelay) { this.successTimeout = successTimeout; this.errorTimeout = errorTimeout; this.initialTimeout = initialTimeout; this.subscribeTimeout = subscribeTimeout; - this.unconfiguredDelay = unconfiguredDelay; - this.configuredErrorDelay = configuredErrorDelay; this.fixedDelay = fixedDelay; this.rand = new Random(System.currentTimeMillis()); } @@ -52,16 +46,12 @@ public class TimingValues { long errorTimeout, long initialTimeout, long subscribeTimeout, - long unconfiguredDelay, - long configuredErrorDelay, long fixedDelay, Random rand) { this.successTimeout = successTimeout; this.errorTimeout = errorTimeout; this.initialTimeout = initialTimeout; this.subscribeTimeout = subscribeTimeout; - this.unconfiguredDelay = unconfiguredDelay; - this.configuredErrorDelay = configuredErrorDelay; this.fixedDelay = fixedDelay; this.rand = rand; } @@ -71,8 +61,6 @@ public class TimingValues { tv.errorTimeout, tv.initialTimeout, tv.subscribeTimeout, - tv.unconfiguredDelay, - tv.configuredErrorDelay, tv.fixedDelay, random); } @@ -115,39 +103,6 @@ public class TimingValues { } /** - * Returns time to wait until next attempt to get config after a failed request when the client has not - * gotten a successful response to a config subscription (i.e, the client has not been configured). - * A negative value means that there will never be a next attempt. If a negative value is set, the - * user must also setSubscribeTimeout(0) to prevent a deadlock while subscribing. - * - * @return delay in milliseconds, a negative value means never. - */ - public long getUnconfiguredDelay() { - return unconfiguredDelay; - } - - public TimingValues setUnconfiguredDelay(long d) { - unconfiguredDelay = d; - return this; - } - - /** - * Returns time to wait until next attempt to get config after a failed request when the client has - * previously gotten a successful response to a config subscription (i.e, the client is configured). - * A negative value means that there will never be a next attempt. - * - * @return delay in milliseconds, a negative value means never. - */ - public long getConfiguredErrorDelay() { - return configuredErrorDelay; - } - - public TimingValues setConfiguredErrorDelay(long d) { - configuredErrorDelay = d; - return this; - } - - /** * Returns fixed delay that is used when retrying getting config no matter if it was a success or an error * and independent of number of retries. * @@ -157,6 +112,11 @@ public class TimingValues { return fixedDelay; } + public TimingValues setFixedDelay(long t) { + fixedDelay = t; + return this; + } + /** * Returns a number +/- a random component * @@ -168,16 +128,6 @@ public class TimingValues { return Math.round(value - (value * fraction) + (rand.nextFloat() * 2L * value * fraction)); } - /** - * Returns a number between 0 and maxValue - * - * @param maxValue max maxValue - * @return a number - */ - public long getRandomTransientDelay(long maxValue) { - return Math.round(rand.nextFloat() * maxValue); - } - @Override public String toString() { return "TimingValues [successTimeout=" + successTimeout @@ -186,8 +136,6 @@ public class TimingValues { + ", subscribeTimeout=" + subscribeTimeout + ", configuredErrorTimeout=" + configuredErrorTimeout + ", fixedDelay=" + fixedDelay - + ", unconfiguredDelay=" + unconfiguredDelay - + ", configuredErrorDelay=" + configuredErrorDelay + ", rand=" + rand + "]"; } diff --git a/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java b/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java index 919155a3944..e60c84df887 100644 --- a/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java +++ b/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java @@ -1,4 +1,4 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.subscription.impl; import com.yahoo.config.subscription.ConfigSourceSet; @@ -20,7 +20,6 @@ import java.util.Random; import static com.yahoo.config.subscription.impl.JRTConfigRequester.calculateFailedRequestDelay; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertSame; @@ -34,59 +33,23 @@ public class JRTConfigRequesterTest { @Test public void testDelayCalculation() { TimingValues defaultTimingValues = new TimingValues(); - Random random = new Random(0); // Use seed to make tests predictable + Random random = new Random(0); // Use seed to make delays deterministic TimingValues timingValues = new TimingValues(defaultTimingValues, random); - // transientFailures and fatalFailures are not set until after delay has been calculated, - // so false is the case for the first failure - boolean fatalFailures = false; - boolean configured = false; - - // First time failure, not configured - long delay = calculateFailedRequestDelay(ErrorType.TRANSIENT, fatalFailures, timingValues, configured); - assertTransientDelay(timingValues.getUnconfiguredDelay(), delay); - delay = calculateFailedRequestDelay(ErrorType.TRANSIENT, fatalFailures, timingValues, configured); - assertTransientDelay(timingValues.getUnconfiguredDelay(), delay); - - - delay = calculateFailedRequestDelay(ErrorType.FATAL, fatalFailures, timingValues, configured); - assertTrue("delay=" + delay, delay > (1 - JRTConfigRequester.randomFraction) * timingValues.getFixedDelay()); - assertTrue("delay=" + delay,delay < (1 + JRTConfigRequester.randomFraction) * timingValues.getFixedDelay()); - assertEquals(4481, delay); - - // First time failure, configured - configured = true; - delay = calculateFailedRequestDelay(ErrorType.TRANSIENT, fatalFailures, timingValues, configured); - assertTransientDelay(timingValues.getConfiguredErrorDelay(), delay); - - delay = calculateFailedRequestDelay(ErrorType.FATAL, fatalFailures, timingValues, configured); - assertTrue(delay > (1 - JRTConfigRequester.randomFraction) * timingValues.getFixedDelay()); - assertTrue(delay < (1 + JRTConfigRequester.randomFraction) * timingValues.getFixedDelay()); - assertEquals(5275, delay); - - - // nth time failure, not configured - fatalFailures = true; - configured = false; - delay = calculateFailedRequestDelay(ErrorType.TRANSIENT, fatalFailures, timingValues, configured); - assertTransientDelay(timingValues.getUnconfiguredDelay(), delay); - delay = calculateFailedRequestDelay(ErrorType.FATAL, fatalFailures, timingValues, configured); - final long l = timingValues.getFixedDelay() + timingValues.getUnconfiguredDelay(); - assertTrue(delay > (1 - JRTConfigRequester.randomFraction) * l); - assertTrue(delay < (1 + JRTConfigRequester.randomFraction) * l); - assertEquals(6121, delay); - - - // nth time failure, configured - fatalFailures = true; - configured = true; - delay = calculateFailedRequestDelay(ErrorType.TRANSIENT, fatalFailures, timingValues, configured); - assertTransientDelay(timingValues.getConfiguredErrorDelay(), delay); - delay = calculateFailedRequestDelay(ErrorType.FATAL, fatalFailures, timingValues, configured); - final long l1 = timingValues.getFixedDelay() + timingValues.getConfiguredErrorDelay(); - assertTrue(delay > (1 - JRTConfigRequester.randomFraction) * l1); - assertTrue(delay < (1 + JRTConfigRequester.randomFraction) * l1); - assertEquals(20780, delay); + int failures = 1; + // First time failure + long delay = calculateFailedRequestDelay(failures, timingValues); + assertEquals(10924, delay); + + failures++; + // 2nd time failure + delay = calculateFailedRequestDelay(failures, timingValues); + assertEquals(22652, delay); + + failures++; + // 3rd time failure + delay = calculateFailedRequestDelay(failures, timingValues); + assertEquals(35849, delay); } @Test @@ -120,7 +83,7 @@ public class JRTConfigRequesterTest { JRTServerConfigRequestV3 receivedRequest = JRTServerConfigRequestV3.createFromRequest(request); assertTrue(receivedRequest.validateParameters()); assertEquals(timingValues.getSubscribeTimeout(), receivedRequest.getTimeout()); - assertFalse(requester.getFatalFailures()); + assertEquals(0, requester.getFailures()); } @Test @@ -132,7 +95,7 @@ public class JRTConfigRequesterTest { JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues); requester.request(createSubscription(subscriber, timingValues)); waitUntilResponse(connection); - assertTrue(requester.getFatalFailures()); + assertEquals(1, requester.getFailures()); } @Test @@ -146,7 +109,7 @@ public class JRTConfigRequesterTest { JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues); requester.request(sub); waitUntilResponse(connection); - assertTrue(requester.getFatalFailures()); + assertEquals(1, requester.getFailures()); } @Test @@ -158,7 +121,7 @@ public class JRTConfigRequesterTest { JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues); requester.request(createSubscription(subscriber, timingValues)); waitUntilResponse(connection); - assertFalse(requester.getFatalFailures()); + assertEquals(1, requester.getFailures()); } @Test @@ -172,7 +135,7 @@ public class JRTConfigRequesterTest { JRTConfigRequester requester = new JRTConfigRequester(connection, timingValues); requester.request(sub); waitUntilResponse(connection); - assertFalse(requester.getFatalFailures()); + assertEquals(1, requester.getFailures()); } @Test @@ -187,7 +150,7 @@ public class JRTConfigRequesterTest { assertEquals(requester.getConnectionPool(), connection); requester.request(sub); waitUntilResponse(connection); - assertTrue(requester.getFatalFailures()); + assertEquals(1, requester.getFailures()); } @Test @@ -257,8 +220,6 @@ public class JRTConfigRequesterTest { 500, // errorTimeout 500, // initialTimeout 2000, // subscribeTimeout - 250, // unconfiguredDelay - 500, // configuredErrorDelay 250); // fixedDelay } @@ -321,10 +282,4 @@ public class JRTConfigRequesterTest { requester2.close(); } - private void assertTransientDelay(long maxDelay, long delay) { - long minDelay = 0; - assertTrue("delay=" + delay + ", minDelay=" + minDelay + ",maxDelay=" + maxDelay, - delay >= minDelay && delay <= maxDelay); - } - } 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 f4c805bd976..badfb3ad931 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 @@ -182,6 +182,7 @@ public class ModelContextImpl implements ModelContext { private final boolean requireConnectivityCheck; private final int maxConcurrentMergesPerContentNode; private final int maxMergeQueueSize; + private final boolean ignoreMergeQueueLimit; private final int largeRankExpressionLimit; private final double resourceLimitDisk; private final double resourceLimitMemory; @@ -213,6 +214,7 @@ public class ModelContextImpl implements ModelContext { this.requireConnectivityCheck = flagValue(source, appId, Flags.REQUIRE_CONNECTIVITY_CHECK); this.maxConcurrentMergesPerContentNode = flagValue(source, appId, Flags.MAX_CONCURRENT_MERGES_PER_NODE); this.maxMergeQueueSize = flagValue(source, appId, Flags.MAX_MERGE_QUEUE_SIZE); + this.ignoreMergeQueueLimit = flagValue(source, appId, Flags.IGNORE_MERGE_QUEUE_LIMIT); this.resourceLimitDisk = flagValue(source, appId, PermanentFlags.RESOURCE_LIMIT_DISK); this.resourceLimitMemory = flagValue(source, appId, PermanentFlags.RESOURCE_LIMIT_MEMORY); this.minNodeRatioPerGroup = flagValue(source, appId, Flags.MIN_NODE_RATIO_PER_GROUP); @@ -245,6 +247,7 @@ public class ModelContextImpl implements ModelContext { @Override public boolean requireConnectivityCheck() { return requireConnectivityCheck; } @Override public int maxConcurrentMergesPerNode() { return maxConcurrentMergesPerContentNode; } @Override public int maxMergeQueueSize() { return maxMergeQueueSize; } + @Override public boolean ignoreMergeQueueLimit() { return ignoreMergeQueueLimit; } @Override public double resourceLimitDisk() { return resourceLimitDisk; } @Override public double resourceLimitMemory() { return resourceLimitMemory; } @Override public double minNodeRatioPerGroup() { return minNodeRatioPerGroup; } diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/ThreadedRequestHandler.java b/container-core/src/main/java/com/yahoo/container/jdisc/ThreadedRequestHandler.java index 446ee90c205..ea686b7b956 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/ThreadedRequestHandler.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/ThreadedRequestHandler.java @@ -174,7 +174,7 @@ public abstract class ThreadedRequestHandler extends AbstractRequestHandler { RequestTask(Request request, BufferedContentChannel content, ResponseHandler responseHandler) { this.request = request; - this.requestReference = request.refer(); + this.requestReference = request.refer(this); this.content = content; this.responseHandler = responseHandler; } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/AsyncCompleteListener.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/AsyncCompleteListener.java deleted file mode 100644 index 7dba217e01c..00000000000 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/AsyncCompleteListener.java +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.jdisc.http.server.jetty; - -import javax.servlet.AsyncEvent; -import javax.servlet.AsyncListener; -import java.io.IOException; - -/** - * Interface for async listeners only interested in onComplete. - * @author Tony Vaagenes - */ -@FunctionalInterface -interface AsyncCompleteListener extends AsyncListener { - @Override - default void onTimeout(AsyncEvent event) throws IOException {} - - @Override - default void onError(AsyncEvent event) throws IOException {} - - @Override - default void onStartAsync(AsyncEvent event) throws IOException {} -} diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FormPostRequestHandler.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FormPostRequestHandler.java index 57fb32f89f0..5ee4434fd84 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FormPostRequestHandler.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/FormPostRequestHandler.java @@ -72,7 +72,7 @@ class FormPostRequestHandler extends AbstractRequestHandler implements ContentCh this.contentCharset = getCharsetByName(contentCharsetName); this.responseHandler = responseHandler; this.request = (HttpRequest) request; - this.requestReference = request.refer(); + this.requestReference = request.refer(this); return this; } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java index 0b7e27b9598..512d78d4537 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java @@ -59,6 +59,8 @@ class HttpRequestDispatch { private final ServletResponseController servletResponseController; private final RequestHandler requestHandler; private final RequestMetricReporter metricReporter; + private final BiConsumer<Void, Throwable> completeRequestCallback; + private final AtomicBoolean completeRequestCalled = new AtomicBoolean(false); public HttpRequestDispatch(JDiscContext jDiscContext, AccessLogEntry accessLogEntry, @@ -80,6 +82,7 @@ class HttpRequestDispatch { this.async = servletRequest.startAsync(); async.setTimeout(0); metricReporter.uriLength(jettyRequest.getOriginalURI().length()); + completeRequestCallback = this::handleCompleteRequestCallback; } public void dispatch() throws IOException { @@ -103,48 +106,44 @@ class HttpRequestDispatch { } } - private final BiConsumer<Void, Throwable> completeRequestCallback; + + private void handleCompleteRequestCallback(Void result, Throwable error) { - AtomicBoolean completeRequestCalled = new AtomicBoolean(false); - HttpRequestDispatch parent = this; //used to avoid binding uninitialized variables - - completeRequestCallback = (result, error) -> { - boolean alreadyCalled = completeRequestCalled.getAndSet(true); - if (alreadyCalled) { - AssertionError e = new AssertionError("completeRequest called more than once"); - log.log(Level.WARNING, "Assertion failed.", e); - throw e; - } + boolean alreadyCalled = completeRequestCalled.getAndSet(true); + if (alreadyCalled) { + AssertionError e = new AssertionError("completeRequest called more than once"); + log.log(Level.WARNING, "Assertion failed.", e); + throw e; + } - boolean reportedError = false; - - if (error != null) { - if (isErrorOfType(error, EofException.class, IOException.class)) { - log.log(Level.FINE, - error, - () -> "Network connection was unexpectedly terminated: " + parent.jettyRequest.getRequestURI()); - parent.metricReporter.prematurelyClosed(); - } else if (isErrorOfType(error, TimeoutException.class)) { - log.log(Level.FINE, - error, - () -> "Request/stream was timed out by Jetty: " + parent.jettyRequest.getRequestURI()); - } else if (!isErrorOfType(error, OverloadException.class, BindingNotFoundException.class, RequestException.class)) { - log.log(Level.WARNING, "Request failed: " + parent.jettyRequest.getRequestURI(), error); - } - reportedError = true; - parent.metricReporter.failedResponse(); - } else { - parent.metricReporter.successfulResponse(); + boolean reportedError = false; + + if (error != null) { + if (isErrorOfType(error, EofException.class, IOException.class)) { + log.log(Level.FINE, + error, + () -> "Network connection was unexpectedly terminated: " + jettyRequest.getRequestURI()); + metricReporter.prematurelyClosed(); + } else if (isErrorOfType(error, TimeoutException.class)) { + log.log(Level.FINE, + error, + () -> "Request/stream was timed out by Jetty: " + jettyRequest.getRequestURI()); + } else if (!isErrorOfType(error, OverloadException.class, BindingNotFoundException.class, RequestException.class)) { + log.log(Level.WARNING, "Request failed: " + jettyRequest.getRequestURI(), error); } + reportedError = true; + metricReporter.failedResponse(); + } else { + metricReporter.successfulResponse(); + } - try { - parent.async.complete(); - log.finest(() -> "Request completed successfully: " + parent.jettyRequest.getRequestURI()); - } catch (Throwable throwable) { - Level level = reportedError ? Level.FINE: Level.WARNING; - log.log(level, "Async.complete failed", throwable); - } - }; + try { + async.complete(); + log.finest(() -> "Request completed successfully: " + jettyRequest.getRequestURI()); + } catch (Throwable throwable) { + Level level = reportedError ? Level.FINE: Level.WARNING; + log.log(level, "Async.complete failed", throwable); + } } private static void shutdownConnectionGracefullyIfThresholdReached(Request request) { diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ReferenceCountingRequestHandler.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ReferenceCountingRequestHandler.java index 71cca62ce9c..00f8fa5140a 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ReferenceCountingRequestHandler.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ReferenceCountingRequestHandler.java @@ -71,6 +71,11 @@ class ReferenceCountingRequestHandler implements DelegatedRequestHandler { } @Override + public ResourceReference refer(Object context) { + return delegate.refer(context); + } + + @Override public void release() { delegate.release(); } @@ -97,7 +102,7 @@ class ReferenceCountingRequestHandler implements DelegatedRequestHandler { Objects.requireNonNull(delegate, "delegate"); this.request = request; this.delegate = delegate; - this.requestReference = request.refer(); + this.requestReference = request.refer(this); } @Override @@ -143,7 +148,7 @@ class ReferenceCountingRequestHandler implements DelegatedRequestHandler { Objects.requireNonNull(delegate, "delegate"); this.request = request; this.delegate = delegate; - this.requestReference = request.refer(); + this.requestReference = request.refer(this); } @Override @@ -200,7 +205,7 @@ class ReferenceCountingRequestHandler implements DelegatedRequestHandler { public ReferenceCountingCompletionHandler(SharedResource request, CompletionHandler delegate) { this.delegate = delegate; - this.requestReference = request.refer(); + this.requestReference = request.refer(this); } @Override diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/HttpRequestTestCase.java b/container-core/src/test/java/com/yahoo/jdisc/http/HttpRequestTestCase.java index 43fc67cfabe..8cd19339d34 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/HttpRequestTestCase.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/HttpRequestTestCase.java @@ -202,6 +202,7 @@ public class HttpRequestTestCase { private static CurrentContainer mockContainer() { final CurrentContainer currentContainer = mock(CurrentContainer.class); when(currentContainer.newReference(any(URI.class))).thenReturn(mock(Container.class)); + when(currentContainer.newReference(any(URI.class), any(Object.class))).thenReturn(mock(Container.class)); return currentContainer; } } diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/HttpResponseTestCase.java b/container-core/src/test/java/com/yahoo/jdisc/http/HttpResponseTestCase.java index 61499200f3c..8cf74749f8d 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/HttpResponseTestCase.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/HttpResponseTestCase.java @@ -134,6 +134,7 @@ public class HttpResponseTestCase { private static CurrentContainer mockContainer() { final CurrentContainer currentContainer = mock(CurrentContainer.class); when(currentContainer.newReference(any(URI.class))).thenReturn(mock(Container.class)); + when(currentContainer.newReference(any(URI.class), any(Object.class))).thenReturn(mock(Container.class)); return currentContainer; } } diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/filter/EmptyRequestFilterTestCase.java b/container-core/src/test/java/com/yahoo/jdisc/http/filter/EmptyRequestFilterTestCase.java index f4418e74169..325a61ee8c1 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/filter/EmptyRequestFilterTestCase.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/filter/EmptyRequestFilterTestCase.java @@ -43,6 +43,7 @@ public class EmptyRequestFilterTestCase { final Method method, final String uri, final Version version) { final CurrentContainer currentContainer = mock(CurrentContainer.class); when(currentContainer.newReference(any(URI.class))).thenReturn(mock(Container.class)); + when(currentContainer.newReference(any(URI.class), any(Object.class))).thenReturn(mock(Container.class)); return HttpRequest.newServerRequest(currentContainer, URI.create(uri), method, version); } } diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/filter/EmptyResponseFilterTestCase.java b/container-core/src/test/java/com/yahoo/jdisc/http/filter/EmptyResponseFilterTestCase.java index e6d7259ea41..d4978e6661d 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/filter/EmptyResponseFilterTestCase.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/filter/EmptyResponseFilterTestCase.java @@ -40,6 +40,7 @@ public class EmptyResponseFilterTestCase { private static HttpRequest newRequest(final Method method, final String uri, final Version version) { final CurrentContainer currentContainer = mock(CurrentContainer.class); when(currentContainer.newReference(any(URI.class))).thenReturn(mock(Container.class)); + when(currentContainer.newReference(any(URI.class), any(Object.class))).thenReturn(mock(Container.class)); return HttpRequest.newServerRequest(currentContainer, URI.create(uri), method, version); } } diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json index e4c1a561dee..4913161d955 100644 --- a/container-search/abi-spec.json +++ b/container-search/abi-spec.json @@ -7076,6 +7076,7 @@ "methods": [ "public void <init>(com.fasterxml.jackson.core.JsonGenerator, boolean)", "public void <init>(com.fasterxml.jackson.core.JsonGenerator, boolean, boolean)", + "public void <init>(com.fasterxml.jackson.core.JsonGenerator, boolean, boolean, boolean)", "public void accept(java.lang.String, java.lang.Object)", "public void accept(java.lang.String, byte[], int, int)", "protected boolean shouldRender(java.lang.String, java.lang.Object)", diff --git a/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java b/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java index ee0e7f4fe0e..92cdd10616b 100644 --- a/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java +++ b/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java @@ -76,6 +76,7 @@ import static com.fasterxml.jackson.databind.SerializationFeature.FLUSH_AFTER_WR // NOTE: The JSON format is a public API. If new elements are added be sure to update the reference doc. public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { + private static final CompoundName WRAP_ALL_MAPS = new CompoundName("renderer.json.jsonMaps"); private static final CompoundName DEBUG_RENDERING_KEY = new CompoundName("renderer.json.debug"); private static final CompoundName JSON_CALLBACK = new CompoundName("jsoncallback"); private static final CompoundName TENSOR_FORMAT = new CompoundName("format.tensors"); @@ -125,6 +126,7 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { private FieldConsumer fieldConsumer; private Deque<Integer> renderedChildren; private boolean debugRendering; + private boolean jsonMaps; private LongSupplier timeSource; private OutputStream stream; @@ -159,6 +161,7 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { public void init() { super.init(); debugRendering = false; + jsonMaps = false; setGenerator(null, debugRendering); renderedChildren = null; timeSource = System::currentTimeMillis; @@ -169,6 +172,7 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { public void beginResponse(OutputStream stream) throws IOException { beginJsonCallback(stream); debugRendering = getDebugRendering(getResult().getQuery()); + jsonMaps = getWrapAllMaps(getResult().getQuery()); tensorShortFormRendering = getTensorShortFormRendering(getResult().getQuery()); setGenerator(generatorFactory.createGenerator(stream, JsonEncoding.UTF8), debugRendering); renderedChildren = new ArrayDeque<>(); @@ -200,6 +204,10 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { generator.writeEndObject(); } + private boolean getWrapAllMaps(Query q) { + return q != null && q.properties().getBoolean(WRAP_ALL_MAPS, false); + } + private boolean getDebugRendering(Query q) { return q != null && q.properties().getBoolean(DEBUG_RENDERING_KEY, false); } @@ -514,11 +522,15 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { private void setGenerator(JsonGenerator generator, boolean debugRendering) { this.generator = generator; - this.fieldConsumer = generator == null ? null : createFieldConsumer(generator, debugRendering); + this.fieldConsumer = generator == null ? null : createFieldConsumer(generator, debugRendering, jsonMaps); } protected FieldConsumer createFieldConsumer(JsonGenerator generator, boolean debugRendering) { - return new FieldConsumer(generator, debugRendering, tensorShortFormRendering); + return createFieldConsumer(generator, debugRendering, this.jsonMaps); + } + + private FieldConsumer createFieldConsumer(JsonGenerator generator, boolean debugRendering, boolean jsonMaps) { + return new FieldConsumer(generator, debugRendering, tensorShortFormRendering, jsonMaps); } /** @@ -537,6 +549,7 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { private final JsonGenerator generator; private final boolean debugRendering; + private final boolean jsonMaps; private final boolean tensorShortForm; private MutableBoolean hasFieldsField; @@ -544,11 +557,14 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { public FieldConsumer(JsonGenerator generator, boolean debugRendering) { this(generator, debugRendering, false); } - public FieldConsumer(JsonGenerator generator, boolean debugRendering, boolean tensorShortForm) { + this(generator, debugRendering, tensorShortForm, false); + } + public FieldConsumer(JsonGenerator generator, boolean debugRendering, boolean tensorShortForm, boolean jsonMaps) { this.generator = generator; this.debugRendering = debugRendering; this.tensorShortForm = tensorShortForm; + this.jsonMaps = jsonMaps; } /** @@ -618,6 +634,43 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { return true; } + private static Inspector deepWrapAsMap(Inspector data) { + if (data.type() == Type.ARRAY) { + var map = new Value.ObjectValue(); + for (int i = 0; i < data.entryCount(); i++) { + Inspector obj = data.entry(i); + if (map != null && obj.type() == Type.OBJECT && obj.fieldCount() == 2) { + Inspector key = obj.field("key"); + Inspector value = obj.field("value"); + if (key.type() == Type.STRING && value.valid()) { + map.put(key.asString(), deepWrapAsMap(value)); + } else { + map = null; + } + } else { + map = null; + } + } + if (map != null) { + return map; + } + var array = new Value.ArrayValue(); + for (int i = 0; i < data.entryCount(); i++) { + Inspector obj = data.entry(i); + array.add(deepWrapAsMap(obj)); + } + return array; + } + if (data.type() == Type.OBJECT) { + var object = new Value.ObjectValue(); + for (var entry : data.fields()) { + object.put(entry.getKey(), deepWrapAsMap(entry.getValue())); + } + return object; + } + return data; + } + private static Inspector wrapAsMap(Inspector data) { if (data.type() != Type.ARRAY) return null; if (data.entryCount() == 0) return null; @@ -636,11 +689,9 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { } private void renderInspector(Inspector data) throws IOException { - Inspector asMap = wrapAsMap(data); + Inspector asMap = jsonMaps ? deepWrapAsMap(data) : wrapAsMap(data); if (asMap != null) { - StringBuilder intermediate = new StringBuilder(); - JsonRender.render(asMap, intermediate, true); - generator.writeRawValue(intermediate.toString()); + renderInspectorDirect(asMap); } else { renderInspectorDirect(data); } diff --git a/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java b/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java index 290b7266a3a..2a399ac0e7b 100644 --- a/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java @@ -1259,6 +1259,54 @@ public class JsonRendererTestCase { assertEqualJson(expected, summary); } + private static SlimeAdapter dataFromSimplified(String simplified) { + var decoder = new com.yahoo.slime.JsonDecoder(); + var slime = decoder.decode(new Slime(), Utf8.toBytes(simplified)); + return new SlimeAdapter(slime.get()); + } + + @Test + public void testMapDeepInFields() throws IOException, InterruptedException, ExecutionException { + Result r = new Result(new Query("/?renderer.json.jsonMaps=true")); + var expected = dataFromSimplified( + "{root: { id:'toplevel', relevance:1.0, fields: { totalCount: 1 }," + + " children: [ { id: 'myHitName', relevance: 1.0," + + " fields: { " + + " f1: [ 'v1', { mykey1: 'myvalue1', mykey2: 'myvalue2' } ]," + + " f2: { i1: 'v2', i2: { mykey3: 'myvalue3' }, i3: 'v3' }," + + " f3: { j1: 42, j2: 17.75, j3: [ 'v4', 'v5' ] }," + + " f4: { mykey4: 'myvalue4', mykey5: 'myvalue5' }" + + " }" + + " } ]" + + "}}"); + Hit h = new Hit("myHitName"); + h.setField("f1", dataFromSimplified("[ 'v1', [ { key: 'mykey1', value: 'myvalue1' }, { key: 'mykey2', value: 'myvalue2' } ] ]")); + h.setField("f2", dataFromSimplified("{ i1: 'v2', i2: [ { key: 'mykey3', value: 'myvalue3' } ], i3: 'v3' }")); + h.setField("f3", dataFromSimplified("{ j1: 42, j2: 17.75, j3: [ 'v4', 'v5' ] }")); + h.setField("f4", dataFromSimplified("[ { key: 'mykey4', value: 'myvalue4' }, { key: 'mykey5', value: 'myvalue5' } ]")); + r.hits().add(h); + r.setTotalHitCount(1L); + String summary = render(r); + assertEqualJson(expected.toString(), summary); + + r = new Result(new Query("/?renderer.json.jsonMaps=false")); + expected = dataFromSimplified( + "{root:{id:'toplevel',relevance:1.0,fields:{totalCount:1}," + + " children: [ { id: 'myHitName', relevance: 1.0," + + " fields: { " + + " f1: [ 'v1', [ { key: 'mykey1', value: 'myvalue1' }, { key: 'mykey2', value: 'myvalue2' } ] ]," + + " f2: { i1: 'v2', i2: [ { key: 'mykey3', value: 'myvalue3' } ], i3: 'v3' }," + + " f3: { j1: 42, j2: 17.75, j3: [ 'v4', 'v5' ] }," + + " f4: { mykey4: 'myvalue4', mykey5: 'myvalue5' }" + + " }" + + " } ]" + + "}}"); + r.hits().add(h); + r.setTotalHitCount(1L); + summary = render(r); + assertEqualJson(expected.toString(), summary); + } + @Test public void testThatTheJsonValidatorCanCatchErrors() { String json = "{" diff --git a/container-search/src/test/java/com/yahoo/search/searchers/test/ConnectionControlSearcherTestCase.java b/container-search/src/test/java/com/yahoo/search/searchers/test/ConnectionControlSearcherTestCase.java index 67dc72c5d7f..b58c4a1ab31 100644 --- a/container-search/src/test/java/com/yahoo/search/searchers/test/ConnectionControlSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/searchers/test/ConnectionControlSearcherTestCase.java @@ -1,7 +1,11 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.searchers.test; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.io.ByteArrayInputStream; import java.net.SocketAddress; @@ -9,7 +13,6 @@ import java.net.URI; import java.net.URISyntaxException; import org.junit.Test; -import org.mockito.Mockito; import com.yahoo.component.chain.Chain; import com.yahoo.container.jdisc.HttpRequest; @@ -79,19 +82,19 @@ public class ConnectionControlSearcherTestCase { private Result doSearch(URI uri, long connectedAtMillis, long nowMillis) { - SocketAddress remoteAddress = Mockito.mock(SocketAddress.class); + SocketAddress remoteAddress = mock(SocketAddress.class); Version version = Version.HTTP_1_1; Method method = Method.GET; - CurrentContainer container = Mockito.mock(CurrentContainer.class); - Mockito.when(container.newReference(Mockito.any())).thenReturn(Mockito.mock(Container.class)); + CurrentContainer container = mock(CurrentContainer.class); + when(container.newReference(any())).thenReturn(mock(Container.class)); + when(container.newReference(any(URI.class), any(Object.class))).thenReturn(mock(Container.class)); final com.yahoo.jdisc.http.HttpRequest serverRequest = com.yahoo.jdisc.http.HttpRequest .newServerRequest(container, uri, method, version, remoteAddress, connectedAtMillis); HttpRequest incoming = new HttpRequest(serverRequest, new ByteArrayInputStream(new byte[0])); Query query = new Query(incoming); Execution e = new Execution(new Chain<Searcher>(ConnectionControlSearcher.createTestInstance(() -> nowMillis)), Execution.Context.createContextStub()); - Result r = e.search(query); - return r; + return e.search(query); } } diff --git a/filedistribution/pom.xml b/filedistribution/pom.xml index e922d878dd7..39bc725d518 100644 --- a/filedistribution/pom.xml +++ b/filedistribution/pom.xml @@ -22,7 +22,7 @@ <groupId>com.yahoo.vespa</groupId> <artifactId>container-apache-http-client-bundle</artifactId> <version>${project.version}</version> - <scope>provided</scope> + <scope>compile</scope> </dependency> <dependency> <groupId>com.yahoo.vespa</groupId> @@ -67,6 +67,11 @@ <groupId>com.fasterxml.jackson.core</groupId> <artifactId>jackson-databind</artifactId> </dependency> + <dependency> + <groupId>org.slf4j</groupId> + <artifactId>slf4j-api</artifactId> + <scope>compile</scope> + </dependency> </dependencies> <build> diff --git a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/status/FileDistributionStatusClient.java b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/status/FileDistributionStatusClient.java index 1675366fc5e..a9bbfafeaf0 100644 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/status/FileDistributionStatusClient.java +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/status/FileDistributionStatusClient.java @@ -1,19 +1,22 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.filedistribution.status; +import ai.vespa.util.http.hc5.VespaHttpClientBuilder; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import io.airlift.airline.Command; import io.airlift.airline.HelpOption; import io.airlift.airline.Option; import io.airlift.airline.SingleCommand; -import org.apache.http.client.config.RequestConfig; -import org.apache.http.client.methods.CloseableHttpResponse; -import org.apache.http.client.methods.HttpGet; -import org.apache.http.client.utils.URIBuilder; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClientBuilder; -import org.apache.http.util.EntityUtils; +import org.apache.hc.client5.http.classic.methods.HttpGet; +import org.apache.hc.client5.http.config.RequestConfig; +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse; +import org.apache.hc.core5.http.HttpEntity; +import org.apache.hc.core5.http.ParseException; +import org.apache.hc.core5.http.io.entity.EntityUtils; +import org.apache.hc.core5.net.URIBuilder; +import org.apache.hc.core5.util.Timeout; import javax.inject.Inject; import java.io.IOException; @@ -23,6 +26,8 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.Map; +import static org.apache.hc.client5.http.config.RequestConfig.custom; + /** * Tool for getting file distribution status * @@ -67,28 +72,32 @@ public class FileDistributionStatusClient { } private String doHttpRequest() { - int timeoutInMillis = (int) (timeout * 1000); - RequestConfig config = RequestConfig.custom() + Timeout timeoutInMillis = Timeout.ofMilliseconds((long) (timeout * 1000)); + RequestConfig config = custom() .setConnectTimeout(timeoutInMillis) .setConnectionRequestTimeout(timeoutInMillis) - .setSocketTimeout(timeoutInMillis) + .setResponseTimeout(timeoutInMillis) .build(); - CloseableHttpClient httpClient = HttpClientBuilder.create().setDefaultRequestConfig(config).build(); + CloseableHttpClient httpClient = VespaHttpClientBuilder.create().build(); URI statusUri = createStatusApiUri(); if (debug) System.out.println("URI:" + statusUri); try { - CloseableHttpResponse response = httpClient.execute(new HttpGet(statusUri)); - String content = EntityUtils.toString(response.getEntity()); + HttpGet request = new HttpGet(statusUri); + request.addHeader("Connection", "Close"); + request.setConfig(config); + CloseableHttpResponse response = httpClient.execute(request); + HttpEntity entity = response.getEntity(); + String content = EntityUtils.toString(entity); if (debug) System.out.println("response:" + content); - if (response.getStatusLine().getStatusCode() == 200) { + if (response.getCode() == 200) { return content; } else { throw new RuntimeException("Failed to get status for request " + statusUri + ": " + - response.getStatusLine() + ": " + content); + response.getCode() + ": " + content); } - } catch (IOException e) { + } catch (IOException | ParseException e) { throw new RuntimeException(e); } } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index fa7f6eedace..74d187368bc 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -197,6 +197,14 @@ public class Flags { "Takes effect at redeploy", ZONE_ID, APPLICATION_ID); + public static final UnboundBooleanFlag IGNORE_MERGE_QUEUE_LIMIT = defineFeatureFlag( + "ignore-merge-queue-limit", false, + List.of("vekterli", "geirst"), "2021-10-06", "2021-12-01", + "Specifies if merges that are forwarded (chained) from another content node are always " + + "allowed to be enqueued even if the queue is otherwise full.", + "Takes effect at redeploy", + ZONE_ID, APPLICATION_ID); + public static final UnboundIntFlag LARGE_RANK_EXPRESSION_LIMIT = defineIntFlag( "large-rank-expression-limit", 8192, List.of("baldersheim"), "2021-06-09", "2021-11-01", diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/AbstractResource.java b/jdisc_core/src/main/java/com/yahoo/jdisc/AbstractResource.java index 05c3582a541..a5b5dad2583 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/AbstractResource.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/AbstractResource.java @@ -2,16 +2,13 @@ package com.yahoo.jdisc; import com.yahoo.jdisc.handler.RequestHandler; +import com.yahoo.jdisc.refcount.DebugReferencesByContextMap; +import com.yahoo.jdisc.refcount.DebugReferencesWithStack; +import com.yahoo.jdisc.refcount.DestructableResource; +import com.yahoo.jdisc.refcount.ReferencesByCount; import com.yahoo.jdisc.service.ClientProvider; import com.yahoo.jdisc.service.ServerProvider; - -import java.util.Arrays; -import java.util.HashSet; -import java.util.Set; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.logging.Level; -import java.util.logging.Logger; +import com.yahoo.jdisc.refcount.References; /** * This class provides a thread-safe implementation of the {@link SharedResource} interface, and should be used for @@ -22,75 +19,33 @@ import java.util.logging.Logger; */ public abstract class AbstractResource implements SharedResource { - private static final Logger log = Logger.getLogger(AbstractResource.class.getName()); + private static final Debug debug = DEBUG; - private final boolean debug = SharedResource.DEBUG; - private final AtomicInteger refCount; - private final Object monitor; - private final Set<Throwable> activeReferences; - private final ResourceReference initialCreationReference; + private final References references; protected AbstractResource() { - if (!debug) { - this.refCount = new AtomicInteger(1); - this.monitor = null; - this.activeReferences = null; - this.initialCreationReference = new NoDebugResourceReference(this); + DestructableResource destructable = new WrappedResource(this); + if (debug == Debug.SIMPLE) { + references = new DebugReferencesByContextMap(destructable, this); + } else if (debug == Debug.STACK) { + references = new DebugReferencesWithStack(destructable); } else { - this.refCount = null; - this.monitor = new Object(); - this.activeReferences = new HashSet<>(); - final Throwable referenceStack = new Throwable(); - this.activeReferences.add(referenceStack); - this.initialCreationReference = new DebugResourceReference(this, referenceStack); + references = new ReferencesByCount(destructable); } } @Override public final ResourceReference refer() { - if (!debug) { - addRef(1); - return new NoDebugResourceReference(this); - } - - final Throwable referenceStack = new Throwable(); - final String state; - synchronized (monitor) { - if (activeReferences.isEmpty()) { - throw new IllegalStateException("Object is already destroyed, no more new references may be created." - + " State={ " + currentStateDebugWithLock() + " }"); - } - activeReferences.add(referenceStack); - state = currentStateDebugWithLock(); - } - log.log(Level.FINE, referenceStack, () -> - getClass().getName() + "@" + System.identityHashCode(this) + ".refer(): state={ " + state + " }"); - return new DebugResourceReference(this, referenceStack); + return refer(null); } - @Override - public final void release() { - initialCreationReference.close(); + public final ResourceReference refer(Object context) { + return references.refer(context); } - private void removeReferenceStack(final Throwable referenceStack, final Throwable releaseStack) { - final boolean doDestroy; - final String state; - synchronized (monitor) { - final boolean wasThere = activeReferences.remove(referenceStack); - state = currentStateDebugWithLock(); - if (!wasThere) { - throw new IllegalStateException("Reference is already released and can only be released once." - + " reference=" + Arrays.toString(referenceStack.getStackTrace()) - + ". State={ " + state + "}"); - } - doDestroy = activeReferences.isEmpty(); - } - log.log(Level.FINE, releaseStack, - () ->getClass().getName() + "@" + System.identityHashCode(this) + " release: state={ " + state + " }"); - if (doDestroy) { - destroy(); - } + @Override + public final void release() { + references.release(); } /** @@ -100,105 +55,25 @@ public abstract class AbstractResource implements SharedResource { * @return The current value of the reference counter. */ public final int retainCount() { - if (!debug) { - return refCount.get(); - } - - synchronized (monitor) { - return activeReferences.size(); - } + return references.referenceCount(); } /** * <p>This method signals that this AbstractResource can dispose of any internal resources, and commence with shut * down of any internal threads. This will be called once the reference count of this resource reaches zero.</p> */ - protected void destroy() { - - } - - private int addRef(int value) { - while (true) { - int prev = refCount.get(); - if (prev == 0) { - throw new IllegalStateException(getClass().getName() + ".addRef(" + value + "):" - + " Object is already destroyed." - + " Consider toggling the " + SharedResource.SYSTEM_PROPERTY_NAME_DEBUG - + " system property to get debugging assistance with reference tracking."); - } - int next = prev + value; - if (refCount.compareAndSet(prev, next)) { - return next; - } - } - } + protected void destroy() { } /** * Returns a string describing the current state of references in human-friendly terms. May be used for debugging. */ public String currentState() { - if (!debug) { - return "Active references: " + refCount.get() + "." - + " Resource reference debugging is turned off. Consider toggling the " - + SharedResource.SYSTEM_PROPERTY_NAME_DEBUG - + " system property to get debugging assistance with reference tracking."; - } - synchronized (monitor) { - return currentStateDebugWithLock(); - } - } - - private String currentStateDebugWithLock() { - return "Active references: " + makeListOfActiveReferences(); - } - - private String makeListOfActiveReferences() { - final StringBuilder builder = new StringBuilder(); - builder.append("["); - for (final Throwable activeReference : activeReferences) { - builder.append(" "); - builder.append(Arrays.toString(activeReference.getStackTrace())); - } - builder.append(" ]"); - return builder.toString(); - } - - private static class NoDebugResourceReference implements ResourceReference { - private final AbstractResource resource; - private final AtomicBoolean isReleased = new AtomicBoolean(false); - - public NoDebugResourceReference(final AbstractResource resource) { - this.resource = resource; - } - - @Override - public final void close() { - final boolean wasReleasedBefore = isReleased.getAndSet(true); - if (wasReleasedBefore) { - final String message = "Reference is already released and can only be released once." - + " State={ " + resource.currentState() + " }"; - throw new IllegalStateException(message); - } - int refCount = resource.addRef(-1); - if (refCount == 0) { - resource.destroy(); - } - } + return references.currentState(); } - private static class DebugResourceReference implements ResourceReference { + static private class WrappedResource implements DestructableResource { private final AbstractResource resource; - private final Throwable referenceStack; - - public DebugResourceReference(final AbstractResource resource, final Throwable referenceStack) { - this.resource = resource; - this.referenceStack = referenceStack; - } - - @Override - public final void close() { - final Throwable releaseStack = new Throwable(); - resource.removeReferenceStack(referenceStack, releaseStack); - } + WrappedResource(AbstractResource resource) { this.resource = resource; } + @Override public void close() { resource.destroy(); } } } diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/NoopSharedResource.java b/jdisc_core/src/main/java/com/yahoo/jdisc/NoopSharedResource.java index 7f4fe96d769..214840aab63 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/NoopSharedResource.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/NoopSharedResource.java @@ -12,6 +12,10 @@ public class NoopSharedResource implements SharedResource { public final ResourceReference refer() { return References.NOOP_REFERENCE; } + @Override + public final ResourceReference refer(Object context) { + return References.NOOP_REFERENCE; + } @Override public final void release() { diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/ProxyRequestHandler.java b/jdisc_core/src/main/java/com/yahoo/jdisc/ProxyRequestHandler.java index 6e385535e40..4e9e45f2f6e 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/ProxyRequestHandler.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/ProxyRequestHandler.java @@ -63,6 +63,11 @@ class ProxyRequestHandler implements DelegatedRequestHandler { } @Override + public ResourceReference refer(Object context) { + return delegate.refer(context); + } + + @Override public void release() { delegate.release(); } @@ -89,7 +94,7 @@ class ProxyRequestHandler implements DelegatedRequestHandler { Objects.requireNonNull(delegate, "delegate"); this.request = request; this.delegate = delegate; - this.requestReference = request.refer(); + this.requestReference = request.refer(this); } @Override @@ -121,7 +126,7 @@ class ProxyRequestHandler implements DelegatedRequestHandler { Objects.requireNonNull(delegate, "delegate"); this.request = request; this.delegate = delegate; - this.requestReference = request.refer(); + this.requestReference = request.refer(this); } @Override @@ -169,7 +174,7 @@ class ProxyRequestHandler implements DelegatedRequestHandler { public ProxyCompletionHandler(SharedResource request, CompletionHandler delegate) { this.delegate = delegate; - this.requestReference = request.refer(); + this.requestReference = request.refer(this); } @Override diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/Request.java b/jdisc_core/src/main/java/com/yahoo/jdisc/Request.java index e2cde8e806c..a6e6e734e6f 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/Request.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/Request.java @@ -87,7 +87,7 @@ public class Request extends AbstractResource { parentReference = null; serverRequest = isServerRequest; setUri(uri); - container = current.newReference(uri); + container = current.newReference(uri, this); creationTime = container.currentTimeMillis(); } @@ -121,7 +121,7 @@ public class Request extends AbstractResource { creationTime = parent.container().currentTimeMillis(); serverRequest = false; setUri(uri); - parentReference = this.parent.refer(); + parentReference = this.parent.refer(this); } /** Returns the {@link Container} for which this Request was created */ diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/SharedResource.java b/jdisc_core/src/main/java/com/yahoo/jdisc/SharedResource.java index 20656bf7d1d..70bfec36a26 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/SharedResource.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/SharedResource.java @@ -28,10 +28,22 @@ import com.yahoo.jdisc.service.ServerProvider; public interface SharedResource { String SYSTEM_PROPERTY_NAME_DEBUG = "jdisc.debug.resources"; - boolean DEBUG = Boolean.valueOf(System.getProperty(SYSTEM_PROPERTY_NAME_DEBUG)); + enum Debug {NO, SIMPLE, STACK} + Debug DEBUG = valueOfDebug(); + private static Debug valueOfDebug() { + String val = System.getProperty(SYSTEM_PROPERTY_NAME_DEBUG); + if (val != null) { + val = val.toUpperCase(); + if (Boolean.valueOf(val)) return Debug.SIMPLE; + try { + return Debug.valueOf(val); + } catch (IllegalArgumentException e) { } + } + return Debug.NO; + } /** - * <p>Increments the reference count of this resource. You call this method to prevent an object from being + * <p>Creates a reference to this resource. You call this method to prevent an object from being * destroyed until you have finished using it.</p> * * <p>You MUST keep the returned {@link ResourceReference} object and release the reference by calling @@ -40,7 +52,24 @@ public interface SharedResource { * * @see ResourceReference#close() */ - ResourceReference refer(); + default ResourceReference refer() { + return refer(null); + } + + /** + * <p>Creates a reference to this resource. You call this method to prevent an object from being + * destroyed until you have finished using it. You can attach a context that will live as long as the reference.</p> + * + * @param context A context to be associated with the reference. It should give some clue as to who referenced it. + * <p>You MUST keep the returned {@link ResourceReference} object and release the reference by calling + * {@link ResourceReference#close()} on it. A reference created by this method can NOT be released by calling + * {@link #release()}.</p> + * + * @see ResourceReference#close() + */ + default ResourceReference refer(Object context) { + return refer(); + } /** * <p>Releases the "main" reference to this resource (the implicit reference due to creation of the object).</p> diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ActiveContainer.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ActiveContainer.java index a9fd2c747ff..1e76f34939f 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ActiveContainer.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ActiveContainer.java @@ -106,7 +106,7 @@ public class ActiveContainer extends AbstractResource implements CurrentContaine } @Override - public ContainerSnapshot newReference(URI uri) { + public ContainerSnapshot newReference(URI uri, Object context) { String name = bindingSetSelector.select(uri); if (name == null) { throw new NoBindingSetSelectedException(uri); @@ -116,7 +116,7 @@ public class ActiveContainer extends AbstractResource implements CurrentContaine if (serverBindings == null || clientBindings == null) { throw new BindingSetNotFoundException(name); } - return new ContainerSnapshot(this, serverBindings, clientBindings); + return new ContainerSnapshot(this, serverBindings, clientBindings, context); } } diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationLoader.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationLoader.java index f28731a47c7..34fcd2be9ba 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationLoader.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationLoader.java @@ -81,12 +81,12 @@ public class ApplicationLoader implements BootstrapLoader, ContainerActivator, C } @Override - public ContainerSnapshot newReference(URI uri) { + public ContainerSnapshot newReference(URI uri, Object context) { ActiveContainer container = containerRef.get(); if (container == null) { throw new ContainerNotReadyException(); } - return container.newReference(uri); + return container.newReference(uri, context); } @Override diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ContainerSnapshot.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ContainerSnapshot.java index 7b72e95ac09..33bec0ec6dc 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ContainerSnapshot.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ContainerSnapshot.java @@ -28,13 +28,13 @@ class ContainerSnapshot extends AbstractResource implements Container { private final BindingSet<RequestHandler> clientBindings; ContainerSnapshot(ActiveContainer container, BindingSet<RequestHandler> serverBindings, - BindingSet<RequestHandler> clientBindings) + BindingSet<RequestHandler> clientBindings, Object context) { this.timeoutMgr = container.timeoutManager(); this.container = container; this.serverBindings = serverBindings; this.clientBindings = clientBindings; - this.containerReference = container.refer(); + this.containerReference = container.refer(context); } @Override @@ -101,6 +101,11 @@ class ContainerSnapshot extends AbstractResource implements Container { } @Override + public ResourceReference refer(Object context) { + return delegate.refer(context); + } + + @Override public void release() { delegate.release(); } diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/TimeoutManagerImpl.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/TimeoutManagerImpl.java index a6d6e32df06..33ed23c9eee 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/TimeoutManagerImpl.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/TimeoutManagerImpl.java @@ -145,6 +145,11 @@ public class TimeoutManagerImpl { } @Override + public ResourceReference refer(Object context) { + return delegate.refer(context); + } + + @Override public void release() { delegate.release(); } diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/handler/ThreadedRequestHandler.java b/jdisc_core/src/main/java/com/yahoo/jdisc/handler/ThreadedRequestHandler.java index 6e2895f118b..97e68c72658 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/handler/ThreadedRequestHandler.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/handler/ThreadedRequestHandler.java @@ -150,7 +150,7 @@ public abstract class ThreadedRequestHandler extends AbstractRequestHandler { this.request = request; this.content = content; this.responseHandler = responseHandler; - this.requestReference = request.refer(); + this.requestReference = request.refer(this); } @Override diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/refcount/CloseableOnce.java b/jdisc_core/src/main/java/com/yahoo/jdisc/refcount/CloseableOnce.java new file mode 100644 index 00000000000..f5b41c5ee9f --- /dev/null +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/refcount/CloseableOnce.java @@ -0,0 +1,27 @@ +package com.yahoo.jdisc.refcount; + +import com.yahoo.jdisc.ResourceReference; + +import java.util.concurrent.atomic.AtomicBoolean; + +/** + * Ensures that a ResourceReference can only be closed exactly once. + * + * @author baldersheim + */ +abstract class CloseableOnce implements ResourceReference { + private final AtomicBoolean isReleased = new AtomicBoolean(false); + + @Override + public final void close() { + final boolean wasReleasedBefore = isReleased.getAndSet(true); + if (wasReleasedBefore) { + final String message = "Reference is already released and can only be released once." + + " State={ " + getReferences().currentState() + " }"; + throw new IllegalStateException(message); + } + onClose(); + } + abstract void onClose(); + abstract References getReferences(); +} diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/refcount/DebugReferencesByContextMap.java b/jdisc_core/src/main/java/com/yahoo/jdisc/refcount/DebugReferencesByContextMap.java new file mode 100644 index 00000000000..e18980967c8 --- /dev/null +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/refcount/DebugReferencesByContextMap.java @@ -0,0 +1,80 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.refcount; + +import com.yahoo.jdisc.ResourceReference; + +import java.util.HashMap; +import java.util.Map; + +/** + * Does reference counting by putting a unique key together with optional context in map + * Used if system property jdisc.debug.resources=simple/true + * + * @author baldersheim + */ +public class DebugReferencesByContextMap implements References { + private final Map<Object, Object> contextMap = new HashMap<>(); + private final DestructableResource resource; + private final Reference initialReference; + private long contextId = 1; + + public DebugReferencesByContextMap(DestructableResource resource, Object context) { + this.resource = resource; + Long key = 0L; + initialReference = new Reference(this, key); + contextMap.put(key, context); + } + + @Override + public void release() { + initialReference.close(); + } + + @Override + public int referenceCount() { + synchronized (contextMap) { return contextMap.size(); } + } + + @Override + public ResourceReference refer(Object context) { + synchronized (contextMap) { + if (contextMap.isEmpty()) { + throw new IllegalStateException("Object is already destroyed, no more new references may be created." + + " State={ " + currentState() + " }"); + } + Long key = contextId++; + contextMap.put(key, context != null ? context : key); + return new Reference(this, key); + } + } + + private void removeRef(Long key) { + synchronized (contextMap) { + contextMap.remove(key); + if (contextMap.isEmpty()) { + resource.close(); + } + } + } + + @Override + public String currentState() { + synchronized (contextMap) { + return contextMap.toString(); + } + } + + private static class Reference extends CloseableOnce { + private final DebugReferencesByContextMap references; + private final Long key; + + Reference(DebugReferencesByContextMap references, Long key) { + this.references = references; + this.key = key; + } + + @Override final void onClose() { references.removeRef(key); } + @Override + final References getReferences() { return references; } + } +} diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/refcount/DebugReferencesWithStack.java b/jdisc_core/src/main/java/com/yahoo/jdisc/refcount/DebugReferencesWithStack.java new file mode 100644 index 00000000000..db6c266534f --- /dev/null +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/refcount/DebugReferencesWithStack.java @@ -0,0 +1,114 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.refcount; + +import com.yahoo.jdisc.ResourceReference; + +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; +import java.util.logging.Level; +import java.util.logging.Logger; + +/** + * Does reference counting by putting stacktraces in a map together with an optional context. + * Intended only for debugging as it is slow. + * Used if system property jdisc.debug.resources=stack + * + * @author baldersheim + */ +public class DebugReferencesWithStack implements References { + private static final Logger log = Logger.getLogger(DebugReferencesWithStack.class.getName()); + private final Map<Throwable, Object> activeReferences = new HashMap<>(); + private final DestructableResource resource; + private final DebugResourceReference initialreference; + + public DebugReferencesWithStack(DestructableResource resource) { + final Throwable referenceStack = new Throwable(); + this.activeReferences.put(referenceStack, this); + this.resource = resource; + initialreference = new DebugResourceReference(this, referenceStack); + } + + @Override + public void release() { + initialreference.close(); + } + + @Override + public int referenceCount() { + synchronized (activeReferences) { + return activeReferences.size(); + } + } + + @Override + public ResourceReference refer(Object context) { + final Throwable referenceStack = new Throwable(); + synchronized (activeReferences) { + if (activeReferences.isEmpty()) { + throw new IllegalStateException("Object is already destroyed, no more new references may be created." + + " State={ " + currentState() + " }"); + } + activeReferences.put(referenceStack, context); + } + log.log(Level.FINE, referenceStack, () -> + getClass().getName() + "@" + System.identityHashCode(this) + ".refer(): state={ " + currentState() + " }"); + return new DebugResourceReference(this, referenceStack); + } + + private void removeReferenceStack(final Throwable referenceStack, final Throwable releaseStack) { + final boolean doDestroy; + synchronized (activeReferences) { + final boolean wasThere = activeReferences.containsKey(referenceStack); + activeReferences.remove(referenceStack); + if (!wasThere) { + throw new IllegalStateException("Reference is already released and can only be released once." + + " reference=" + Arrays.toString(referenceStack.getStackTrace()) + + ". State={ " + currentState() + "}"); + } + doDestroy = activeReferences.isEmpty(); + log.log(Level.FINE, releaseStack, + () -> getClass().getName() + "@" + System.identityHashCode(this) + " release: state={ " + currentState() + " }"); + } + + if (doDestroy) { + resource.close(); + } + } + + @Override + public String currentState() { + return "Active references: " + makeListOfActiveReferences(); + } + + private String makeListOfActiveReferences() { + final StringBuilder builder = new StringBuilder(); + builder.append("["); + synchronized (activeReferences) { + for (var activeReference : activeReferences.entrySet()) { + builder.append(" "); + builder.append(Arrays.toString(activeReference.getKey().getStackTrace())); + } + } + builder.append(" ]"); + return builder.toString(); + } + + private static class DebugResourceReference extends CloseableOnce { + private final DebugReferencesWithStack resource; + private final Throwable referenceStack; + + public DebugResourceReference(DebugReferencesWithStack resource, final Throwable referenceStack) { + this.resource = resource; + this.referenceStack = referenceStack; + } + + @Override + final void onClose() { + final Throwable releaseStack = new Throwable(); + resource.removeReferenceStack(referenceStack, releaseStack); + } + @Override + final References getReferences() { return resource; } + } +} diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/refcount/DestructableResource.java b/jdisc_core/src/main/java/com/yahoo/jdisc/refcount/DestructableResource.java new file mode 100644 index 00000000000..b373473f214 --- /dev/null +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/refcount/DestructableResource.java @@ -0,0 +1,11 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.refcount; + +public interface DestructableResource extends AutoCloseable { + + /** + * Wrapper to allow access to protected AbstractResource.destroy() + */ + @Override + void close(); +} diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/refcount/References.java b/jdisc_core/src/main/java/com/yahoo/jdisc/refcount/References.java new file mode 100644 index 00000000000..35b4e7c759e --- /dev/null +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/refcount/References.java @@ -0,0 +1,22 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.refcount; + +import com.yahoo.jdisc.ResourceReference; + +/** + * Interface for implementations of reference counting + * @author baldersheim + */ +public interface References { + /** Release the initial reference */ + void release(); + /** Returns number of held references */ + int referenceCount(); + /** + * Adds a reference and return an objects that when closed will return the reference. + * Supply a context that can provide link to the one holding the link. Useful for debugging + */ + ResourceReference refer(Object context); + + String currentState(); +} diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/refcount/ReferencesByCount.java b/jdisc_core/src/main/java/com/yahoo/jdisc/refcount/ReferencesByCount.java new file mode 100644 index 00000000000..0f417c81a8b --- /dev/null +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/refcount/ReferencesByCount.java @@ -0,0 +1,83 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.refcount; + +import com.yahoo.jdisc.ResourceReference; +import com.yahoo.jdisc.SharedResource; + +import java.util.concurrent.atomic.AtomicInteger; + +/** + * Does reference counting by using atomic counting of references + * Default in production + * + * @author baldersheim + */ +public class ReferencesByCount implements References { + private final AtomicInteger refCount; + private final DestructableResource resource; + private final NoDebugResourceReference initialReference; + + public ReferencesByCount(DestructableResource resource) { + refCount = new AtomicInteger(1); + this.resource = resource; + initialReference = new NoDebugResourceReference(this); + } + + @Override + public void release() { + initialReference.close(); + } + + @Override + public int referenceCount() { + return refCount.get(); + } + + @Override + public ResourceReference refer(Object context) { + addRef(1); + return new NoDebugResourceReference(this); + } + + @Override + public String currentState() { + return "Active references: " + refCount.get() + "." + + " Resource reference debugging is turned off. Consider toggling the " + + SharedResource.SYSTEM_PROPERTY_NAME_DEBUG + + " system property to get debugging assistance with reference tracking."; + } + + private void removeRef() { + int refCount = addRef(-1); + if (refCount == 0) { + resource.close(); + } + } + + private int addRef(int value) { + while (true) { + int prev = refCount.get(); + if (prev == 0) { + throw new IllegalStateException(getClass().getName() + ".addRef(" + value + "):" + + " Object is already destroyed." + + " Consider toggling the " + SharedResource.SYSTEM_PROPERTY_NAME_DEBUG + + " system property to get debugging assistance with reference tracking."); + } + int next = prev + value; + if (refCount.compareAndSet(prev, next)) { + return next; + } + } + } + + private static class NoDebugResourceReference extends CloseableOnce { + private final ReferencesByCount resource; + + NoDebugResourceReference(final ReferencesByCount resource) { + this.resource = resource; + } + + @Override final void onClose() { resource.removeRef(); } + @Override References getReferences() { return resource; } + } +} diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/refcount/package-info.java b/jdisc_core/src/main/java/com/yahoo/jdisc/refcount/package-info.java new file mode 100644 index 00000000000..0797951df9e --- /dev/null +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/refcount/package-info.java @@ -0,0 +1,4 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +@com.yahoo.osgi.annotation.ExportPackage +package com.yahoo.jdisc.refcount; diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/service/CurrentContainer.java b/jdisc_core/src/main/java/com/yahoo/jdisc/service/CurrentContainer.java index 9b7af7e49f5..f0a39e0f045 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/service/CurrentContainer.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/service/CurrentContainer.java @@ -27,11 +27,19 @@ public interface CurrentContainer { * * @param uri The identifier used to match this Request to an appropriate {@link ClientProvider} or {@link * RequestHandler}. The hostname must be "localhost" or a fully qualified domain name. + * @param context that can be attached for reference tracking * @return A reference to the current Container. * @throws NoBindingSetSelectedException If no {@link BindingSet} was selected by the {@link BindingSetSelector}. * @throws BindingSetNotFoundException If the named BindingSet was not found. * @throws ContainerNotReadyException If no active Container was found, this can only happen during initial * setup. */ - public Container newReference(URI uri); + default Container newReference(URI uri, Object context) { + return newReference(uri); + } + + default Container newReference(URI uri) { + return newReference(uri, null); + } + } diff --git a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerSnapshotTestCase.java b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerSnapshotTestCase.java index ac2efafbba5..472ac095188 100644 --- a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerSnapshotTestCase.java +++ b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerSnapshotTestCase.java @@ -146,7 +146,7 @@ public class ContainerSnapshotTestCase { } }); ActiveContainer active = new ActiveContainer(driver.newContainerBuilder()); - ContainerSnapshot snapshot = new ContainerSnapshot(active, null, null); + ContainerSnapshot snapshot = new ContainerSnapshot(active, null, null, null); assertSame(obj, snapshot.getInstance(Object.class)); assertEquals("foo", snapshot.getInstance(Key.get(String.class, Names.named("foo")))); snapshot.release(); diff --git a/searchlib/abi-spec.json b/searchlib/abi-spec.json index 2468fd0c5c7..4ebca94734f 100644 --- a/searchlib/abi-spec.json +++ b/searchlib/abi-spec.json @@ -898,6 +898,7 @@ "public final com.yahoo.searchlib.rankingexpression.rule.TensorFunctionNode tensorMatmul()", "public final com.yahoo.searchlib.rankingexpression.rule.TensorFunctionNode tensorSoftmax()", "public final com.yahoo.searchlib.rankingexpression.rule.TensorFunctionNode tensorXwPlusB()", + "public final com.yahoo.searchlib.rankingexpression.rule.TensorFunctionNode tensorExpand()", "public final com.yahoo.searchlib.rankingexpression.rule.TensorFunctionNode tensorArgmax()", "public final com.yahoo.searchlib.rankingexpression.rule.TensorFunctionNode tensorArgmin()", "public final com.yahoo.searchlib.rankingexpression.rule.TensorFunctionNode tensorCellCast()", @@ -1053,6 +1054,7 @@ "public static final int ARGMAX", "public static final int ARGMIN", "public static final int CELL_CAST", + "public static final int EXPAND", "public static final int AVG", "public static final int COUNT", "public static final int MAX", diff --git a/searchlib/src/main/javacc/RankingExpressionParser.jj b/searchlib/src/main/javacc/RankingExpressionParser.jj index 7bfbfd6c005..88eb0feeb73 100755 --- a/searchlib/src/main/javacc/RankingExpressionParser.jj +++ b/searchlib/src/main/javacc/RankingExpressionParser.jj @@ -144,6 +144,7 @@ TOKEN : <ARGMAX: "argmax"> | <ARGMIN: "argmin"> | <CELL_CAST: "cell_cast"> | + <EXPAND: "expand"> | <AVG: "avg" > | <COUNT: "count"> | @@ -384,7 +385,8 @@ TensorFunctionNode tensorFunction() : tensorExpression = tensorXwPlusB() | tensorExpression = tensorArgmax() | tensorExpression = tensorArgmin() | - tensorExpression = tensorCellCast() + tensorExpression = tensorCellCast() | + tensorExpression = tensorExpand() ) { return tensorExpression; } } @@ -581,6 +583,16 @@ TensorFunctionNode tensorXwPlusB() : dimension)); } } +TensorFunctionNode tensorExpand() : +{ + ExpressionNode argument; + String dimension; +} +{ + <EXPAND> <LBRACE> argument = expression() <COMMA> dimension = identifier() <RBRACE> + { return new TensorFunctionNode(new Expand(TensorFunctionNode.wrap(argument), dimension)); } +} + TensorFunctionNode tensorArgmax() : { ExpressionNode tensor; @@ -696,6 +708,7 @@ String tensorFunctionName() : ( <ARGMAX> { return token.image; } ) | ( <ARGMIN> { return token.image; } ) | ( <CELL_CAST> { return token.image; } ) | + ( <EXPAND> { return token.image; } ) | ( aggregator = tensorReduceAggregator() { return aggregator.toString(); } ) } diff --git a/searchlib/src/protobuf/search_protocol.proto b/searchlib/src/protobuf/search_protocol.proto index ded19fe132f..b41c3b65a74 100644 --- a/searchlib/src/protobuf/search_protocol.proto +++ b/searchlib/src/protobuf/search_protocol.proto @@ -51,6 +51,11 @@ message SearchReply { repeated Hit hits = 7; bytes grouping_blob = 8; // serialized opaquely like now, to be changed later bytes slime_trace = 9; + repeated Error errors = 10; +} + +message Error { + string message = 1; } message Hit { @@ -79,6 +84,7 @@ message DocsumRequest { message DocsumReply { bytes slime_summaries = 1; // result array inside slime object + repeated Error errors = 2; } message MonitorRequest { diff --git a/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/EvaluationTestCase.java b/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/EvaluationTestCase.java index 246dbcb2b1e..10c835b05f2 100644 --- a/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/EvaluationTestCase.java +++ b/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/EvaluationTestCase.java @@ -498,7 +498,6 @@ public class EvaluationTestCase { "tensor(d0[3],d1[2],d2[1],d3[1])(tensor0{a0:0, a1:((d0 * 2 + d1) / 3), a2:((d0 * 2 + d1) % 3) })", "tensor(a0[1],a1[2],a2[3]):[1,2,3,4,5,6]", "tensor(d0[4]):[3,2,-1,1]"); - } @Test @@ -725,6 +724,13 @@ public class EvaluationTestCase { tester.assertEvaluates("tensor(d0[1], d1[3]):[1, 2, 3]", "tensor0 * tensor(d0[1])(1)", "tensor(d1[3]):[1, 2, 3]"); + // Add using the "expand" non-primitive function + tester.assertEvaluates("tensor(d0[1],d1[3]):[[1,2,3]]", + "expand(tensor0, d0)", + "tensor(d1[3]):[1, 2, 3]"); + tester.assertEvaluates("tensor<float>(d0[1],d1[3]):[[1,2,3]]", + "expand(tensor0, d0)", + "tensor<float>(d1[3]):[1, 2, 3]"); } @Test diff --git a/vespajlib/abi-spec.json b/vespajlib/abi-spec.json index e68a37b15b6..c426195bc37 100644 --- a/vespajlib/abi-spec.json +++ b/vespajlib/abi-spec.json @@ -1180,6 +1180,7 @@ "public com.yahoo.tensor.Tensor matmul(com.yahoo.tensor.Tensor, java.lang.String)", "public com.yahoo.tensor.Tensor softmax(java.lang.String)", "public com.yahoo.tensor.Tensor xwPlusB(com.yahoo.tensor.Tensor, com.yahoo.tensor.Tensor, java.lang.String)", + "public com.yahoo.tensor.Tensor expand(java.lang.String)", "public com.yahoo.tensor.Tensor argmax(java.lang.String)", "public com.yahoo.tensor.Tensor argmin(java.lang.String)", "public static com.yahoo.tensor.Tensor diag(com.yahoo.tensor.TensorType)", @@ -1699,6 +1700,21 @@ ], "fields": [] }, + "com.yahoo.tensor.functions.Expand": { + "superClass": "com.yahoo.tensor.functions.CompositeTensorFunction", + "interfaces": [], + "attributes": [ + "public" + ], + "methods": [ + "public void <init>(com.yahoo.tensor.functions.TensorFunction, java.lang.String)", + "public java.util.List arguments()", + "public com.yahoo.tensor.functions.TensorFunction withArguments(java.util.List)", + "public com.yahoo.tensor.functions.PrimitiveTensorFunction toPrimitive()", + "public java.lang.String toString(com.yahoo.tensor.functions.ToStringContext)" + ], + "fields": [] + }, "com.yahoo.tensor.functions.Generate": { "superClass": "com.yahoo.tensor.functions.PrimitiveTensorFunction", "interfaces": [], @@ -2053,6 +2069,22 @@ ], "fields": [] }, + "com.yahoo.tensor.functions.ScalarFunctions$Constant": { + "superClass": "java.lang.Object", + "interfaces": [ + "java.util.function.Function" + ], + "attributes": [ + "public" + ], + "methods": [ + "public void <init>(double)", + "public java.lang.Double apply(java.util.List)", + "public java.lang.String toString()", + "public bridge synthetic java.lang.Object apply(java.lang.Object)" + ], + "fields": [] + }, "com.yahoo.tensor.functions.ScalarFunctions$Cos": { "superClass": "java.lang.Object", "interfaces": [ @@ -2603,7 +2635,8 @@ "public static java.util.function.DoubleUnaryOperator selu(double, double)", "public static java.util.function.Function random()", "public static java.util.function.Function equal(java.util.List)", - "public static java.util.function.Function sum(java.util.List)" + "public static java.util.function.Function sum(java.util.List)", + "public static java.util.function.Function constant(double)" ], "fields": [] }, diff --git a/vespajlib/src/main/java/com/yahoo/tensor/Tensor.java b/vespajlib/src/main/java/com/yahoo/tensor/Tensor.java index 3d4536d9249..047844113ff 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/Tensor.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/Tensor.java @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.tensor; -import com.yahoo.tensor.evaluation.TypeContext; import com.yahoo.tensor.functions.Argmax; import com.yahoo.tensor.functions.Argmin; import com.yahoo.tensor.functions.CellCast; @@ -20,7 +19,7 @@ import com.yahoo.tensor.functions.Reduce; import com.yahoo.tensor.functions.Rename; import com.yahoo.tensor.functions.Softmax; import com.yahoo.tensor.functions.XwPlusB; -import com.yahoo.text.Ascii7BitMatcher; +import com.yahoo.tensor.functions.Expand; import java.util.ArrayList; import java.util.Arrays; @@ -35,7 +34,6 @@ import java.util.function.DoubleUnaryOperator; import java.util.function.Function; import java.util.stream.Collectors; -import static com.yahoo.text.Ascii7BitMatcher.charsAndNumbers; import static com.yahoo.tensor.functions.ScalarFunctions.Hamming; /** @@ -210,6 +208,10 @@ public interface Tensor { return new XwPlusB<>(new ConstantTensor<>(this), new ConstantTensor<>(w), new ConstantTensor<>(b), dimension).evaluate(); } + default Tensor expand(String dimension) { + return new Expand<>(new ConstantTensor<>(this), dimension).evaluate(); + } + default Tensor argmax(String dimension) { return new Argmax<>(new ConstantTensor<>(this), dimension).evaluate(); } diff --git a/vespajlib/src/main/java/com/yahoo/tensor/functions/Expand.java b/vespajlib/src/main/java/com/yahoo/tensor/functions/Expand.java new file mode 100644 index 00000000000..8fc246a7d9d --- /dev/null +++ b/vespajlib/src/main/java/com/yahoo/tensor/functions/Expand.java @@ -0,0 +1,48 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.tensor.functions; + +import com.yahoo.tensor.TensorType; +import com.yahoo.tensor.evaluation.Name; + +import java.util.Collections; +import java.util.List; + +/** + * The <i>expand</i> tensor function returns a tensor with a new dimension of + * size 1 is added, equivalent to "tensor * tensor(dim_name[1])(1)". + * + * @author lesters + */ +public class Expand<NAMETYPE extends Name> extends CompositeTensorFunction<NAMETYPE> { + + private final TensorFunction<NAMETYPE> argument; + private final String dimensionName; + + public Expand(TensorFunction<NAMETYPE> argument, String dimension) { + this.argument = argument; + this.dimensionName = dimension; + } + + @Override + public List<TensorFunction<NAMETYPE>> arguments() { return Collections.singletonList(argument); } + + @Override + public TensorFunction<NAMETYPE> withArguments(List<TensorFunction<NAMETYPE>> arguments) { + if (arguments.size() != 1) + throw new IllegalArgumentException("Expand must have 1 argument, got " + arguments.size()); + return new Expand<>(arguments.get(0), dimensionName); + } + + @Override + public PrimitiveTensorFunction<NAMETYPE> toPrimitive() { + TensorType type = new TensorType.Builder(TensorType.Value.INT8).indexed(dimensionName, 1).build(); + Generate<NAMETYPE> expansion = new Generate<>(type, ScalarFunctions.constant(1.0)); + return new Join<>(expansion, argument, ScalarFunctions.multiply()); + } + + @Override + public String toString(ToStringContext context) { + return "expand(" + argument.toString(context) + ", " + dimensionName + ")"; + } + +} diff --git a/vespajlib/src/main/java/com/yahoo/tensor/functions/ScalarFunctions.java b/vespajlib/src/main/java/com/yahoo/tensor/functions/ScalarFunctions.java index d6fcd17b8fb..4c2b64244e5 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/functions/ScalarFunctions.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/functions/ScalarFunctions.java @@ -66,6 +66,7 @@ public class ScalarFunctions { public static Function<List<Long>, Double> random() { return new Random(); } public static Function<List<Long>, Double> equal(List<String> argumentNames) { return new EqualElements(argumentNames); } public static Function<List<Long>, Double> sum(List<String> argumentNames) { return new SumElements(argumentNames); } + public static Function<List<Long>, Double> constant(double value) { return new Constant(value); } // Binary operators ----------------------------------------------------------------------------- @@ -493,4 +494,19 @@ public class ScalarFunctions { } } + public static class Constant implements Function<List<Long>, Double> { + private final double value; + + public Constant(double value) { + this.value = value; + } + @Override + public Double apply(List<Long> values) { + return value; + } + @Override + public String toString() { return Double.toString(value); } + } + + } |