diff options
66 files changed, 868 insertions, 379 deletions
diff --git a/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java b/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java index 6a8d754af1c..8ca0e5c501c 100644 --- a/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java +++ b/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java @@ -258,7 +258,7 @@ public class MockApplicationPackage implements ApplicationPackage { @Override - public void validateXML() throws IOException { + public void validateXML() { if (failOnValidateXml) { throw new IllegalArgumentException("Error in application package"); } else { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java index da0566934f7..6ee525c1246 100755 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java @@ -419,7 +419,8 @@ public final class ContainerCluster @NonNull public SearchChains getSearchChains() { if (containerSearch == null) - throw new IllegalStateException("Null search components!"); + throw new IllegalStateException("Search components not found in container cluster '" + getSubId() + + "': Add <search/> to the cluster in services.xml"); return containerSearch.getChains(); } @@ -479,7 +480,8 @@ public final class ContainerCluster @NonNull public DocprocChains getDocprocChains() { if (containerDocproc == null) - throw new IllegalStateException("Null docproc components!"); + throw new IllegalStateException("Document processing components not found in container cluster '" + getSubId() + + "': Add <document-processing/> to the cluster in services.xml"); return containerDocproc.getChains(); } diff --git a/config-model/src/main/perl/vespa-deploy b/config-model/src/main/perl/vespa-deploy index cb198c170a5..8a74d97e868 100755 --- a/config-model/src/main/perl/vespa-deploy +++ b/config-model/src/main/perl/vespa-deploy @@ -310,13 +310,13 @@ sub get_configsource_url { my $configsource_url = shift(@configsources); if (!$configsource_url) { - die "Could not get url to config server, make sure that VESPA_HOME and services.addr_configserver is set\n"; + die "Could not get url to config server, make sure that VESPA_HOME and VESPA_CONFIGSERVERS are set\n"; } chomp($configsource_url); my @temp = split(':', $configsource_url, 3); $configsource_url = $temp[0] . ":" . $temp[1] . ":" . $port; if (!$configsource_url) { - print "Could not get url to config server, make sure that services.addr_configserver is set\n"; + print "Could not get url to config server, make sure that VESPA_CONFIGSERVERS is set\n"; exit 1; } diff --git a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java index d4ce445d50c..dd5a15acb2a 100644 --- a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java +++ b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java @@ -1357,47 +1357,154 @@ public class ModelProvisioningTest { } @Test + public void testModelWithReferencedIndexingCluster() { + String services = + "<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n" + + "<services version=\"1.0\">\n" + + "\n" + + " <admin version=\"2.0\">\n" + + " <adminserver hostalias=\"vespa-1\"/>\n" + + " <configservers>\n" + + " <configserver hostalias=\"vespa-1\"/>\n" + + " </configservers>\n" + + " </admin>\n" + + "\n" + + " <container id=\"container\" version=\"1.0\">\n" + + " <document-processing/>\n" + + " <document-api/>\n" + + " <search/>\n" + + " <nodes jvmargs=\"-Xms512m -Xmx512m\">\n" + + " <node hostalias=\"vespa-1\"/>\n" + + " </nodes>\n" + + " </container>\n" + + "\n" + + " <content id=\"storage\" version=\"1.0\">\n" + + " <search>\n" + + " <visibility-delay>1.0</visibility-delay>\n" + + " </search>\n" + + " <redundancy>2</redundancy>\n" + + " <documents>\n" + + " <document type=\"type1\" mode=\"index\"/>\n" + + " <document-processing cluster=\"container\"/>\n" + + " </documents>\n" + + " <nodes>\n" + + " <node hostalias=\"vespa-1\" distribution-key=\"0\"/>\n" + + " </nodes>\n" + + " </content>\n" + + "\n" + + "</services>"; + + VespaModel model = createNonProvisionedMultitenantModel(services); + assertThat(model.getRoot().getHostSystem().getHosts().size(), is(1)); + ContentCluster content = model.getContentClusters().get("storage"); + assertEquals(1, content.getRootGroup().getNodes().size()); + ContainerCluster controller = content.getClusterControllers(); + assertEquals(1, controller.getContainers().size()); + } + + @Test + public void testSharedNodesNotHosted() { + String hosts = + "<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n" + + "<hosts>\n" + + " <host name=\"vespa-1\">\n" + + " <alias>vespa-1</alias>\n" + + " </host>\n" + + " <host name=\"vespa-2\">\n" + + " <alias>vespa-2</alias>\n" + + " </host>\n" + + " <host name=\"vespa-3\">\n" + + " <alias>vespa-3</alias>\n" + + " </host>\n" + + "</hosts>"; + String services = + "<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n" + + "<services version=\"1.0\">\n" + + "\n" + + " <admin version=\"2.0\">\n" + + " <adminserver hostalias=\"vespa-1\"/>\n" + + " <configservers>\n" + + " <configserver hostalias=\"vespa-1\"/>\n" + + " </configservers>\n" + + " </admin>\n" + + "\n" + + " <container id=\"container\" version=\"1.0\">\n" + + " <document-processing/>\n" + + " <document-api/>\n" + + " <search/>\n" + + " <nodes jvmargs=\"-Xms512m -Xmx512m\">\n" + + " <node hostalias=\"vespa-1\"/>\n" + + " <node hostalias=\"vespa-2\"/>\n" + + " <node hostalias=\"vespa-3\"/>\n" + + " </nodes>\n" + + " </container>\n" + + "\n" + + " <content id=\"storage\" version=\"1.0\">\n" + + " <search>\n" + + " <visibility-delay>1.0</visibility-delay>\n" + + " </search>\n" + + " <redundancy>2</redundancy>\n" + + " <documents>\n" + + " <document type=\"type1\" mode=\"index\"/>\n" + + " <document-processing cluster=\"container\"/>\n" + + " </documents>\n" + + " <nodes>\n" + + " <node hostalias=\"vespa-1\" distribution-key=\"0\"/>\n" + + " <node hostalias=\"vespa-2\" distribution-key=\"1\"/>\n" + + " <node hostalias=\"vespa-3\" distribution-key=\"2\"/>\n" + + " </nodes>\n" + + " </content>\n" + + "\n" + + "</services>"; + + VespaModel model = createNonProvisionedModel(false, hosts, services); + assertEquals(3, model.getRoot().getHostSystem().getHosts().size()); + ContentCluster content = model.getContentClusters().get("storage"); + assertEquals(3, content.getRootGroup().getNodes().size()); + } + + @Test public void testMultitenantButNotHostedSharedContentNode() { String services = - "<?xml version='1.0' encoding='UTF-8' ?>" + - "<services version='1.0'>" + - " <admin version='2.0'>" + - " <adminserver hostalias='node1'/>" + - " </admin>" + - " <jdisc id='default' version='1.0'>" + - " <search/>" + - " <nodes>" + - " <node hostalias='node1'/>" + - " </nodes>" + - " </jdisc>" + - " <content id='storage' version='1.0'>" + - " <redundancy>2</redundancy>" + - " <group>" + - " <node distribution-key='0' hostalias='node1'/>" + - " <node distribution-key='1' hostalias='node1'/>" + - " </group>" + - " <tuning>" + - " <cluster-controller>" + - " <transition-time>0</transition-time>" + - " </cluster-controller>" + - " </tuning>" + - " <documents>" + - " <document mode='store-only' type='type1'/>" + - " </documents>" + - " <engine>" + - " <proton/>" + - " </engine>" + - " </content>" + - " <content id='search' version='1.0'>" + - " <redundancy>2</redundancy>" + - " <group>" + - " <node distribution-key='0' hostalias='node1'/>" + - " </group>" + - " <documents>" + - " <document type='type1'/>" + - " </documents>" + - " </content>" + - " </services>"; + "<?xml version='1.0' encoding='UTF-8' ?>" + + "<services version='1.0'>" + + " <admin version='2.0'>" + + " <adminserver hostalias='node1'/>" + + " </admin>" + + " <jdisc id='default' version='1.0'>" + + " <search/>" + + " <nodes>" + + " <node hostalias='node1'/>" + + " </nodes>" + + " </jdisc>" + + " <content id='storage' version='1.0'>" + + " <redundancy>2</redundancy>" + + " <group>" + + " <node distribution-key='0' hostalias='node1'/>" + + " <node distribution-key='1' hostalias='node1'/>" + + " </group>" + + " <tuning>" + + " <cluster-controller>" + + " <transition-time>0</transition-time>" + + " </cluster-controller>" + + " </tuning>" + + " <documents>" + + " <document mode='store-only' type='type1'/>" + + " </documents>" + + " <engine>" + + " <proton/>" + + " </engine>" + + " </content>" + + " <content id='search' version='1.0'>" + + " <redundancy>2</redundancy>" + + " <group>" + + " <node distribution-key='0' hostalias='node1'/>" + + " </group>" + + " <documents>" + + " <document type='type1'/>" + + " </documents>" + + " </content>" + + " </services>"; VespaModel model = createNonProvisionedMultitenantModel(services); assertThat(model.getRoot().getHostSystem().getHosts().size(), is(1)); @@ -1408,10 +1515,14 @@ public class ModelProvisioningTest { } private VespaModel createNonProvisionedMultitenantModel(String services) { - VespaModelCreatorWithMockPkg modelCreatorWithMockPkg = new VespaModelCreatorWithMockPkg(null, services, ApplicationPackageUtils.generateSearchDefinition("type1")); + return createNonProvisionedModel(true, null, services); + } + + private VespaModel createNonProvisionedModel(boolean multitenant, String hosts, String services) { + VespaModelCreatorWithMockPkg modelCreatorWithMockPkg = new VespaModelCreatorWithMockPkg(hosts, services, ApplicationPackageUtils.generateSearchDefinition("type1")); ApplicationPackage appPkg = modelCreatorWithMockPkg.appPkg; DeployState deployState = new DeployState.Builder().applicationPackage(appPkg). - properties((new DeployProperties.Builder()).multitenant(true).build()). + properties((new DeployProperties.Builder()).multitenant(multitenant).build()). build(true); return modelCreatorWithMockPkg.create(false, deployState); } diff --git a/config-provisioning/src/main/resources/configdefinitions/node-repository.def b/config-provisioning/src/main/resources/configdefinitions/node-repository.def index 8ea9265aa23..4af8806fbbb 100644 --- a/config-provisioning/src/main/resources/configdefinitions/node-repository.def +++ b/config-provisioning/src/main/resources/configdefinitions/node-repository.def @@ -6,3 +6,6 @@ namespace=config.provisioning dockerImage string default="dummyImage" useCuratorClientCache bool default=false + +# Comma separated list of hostnames that are authorized for all config server REST APIs +hostnameWhitelist string default="" diff --git a/config/src/apps/vespa-config/vespa-config.pl b/config/src/apps/vespa-config/vespa-config.pl index 61a1714873a..735a8054f52 100755 --- a/config/src/apps/vespa-config/vespa-config.pl +++ b/config/src/apps/vespa-config/vespa-config.pl @@ -76,7 +76,6 @@ my $myHostname = `vespa-print-default hostname`; chomp $myHostname; my $default_configproxy_port = "19090"; my $default_configserver_port = "19070"; -my $zk_client_port; my $base_cfg_dir = $VESPA_HOME . "/conf/vespa"; @@ -84,9 +83,6 @@ my $base_cfg_dir = $VESPA_HOME . "/conf/vespa"; # of in environment variables my $lookupInConfig = 0; -sub vespa_base_env { - $zk_client_port = getCCSVar('zookeeper_clientPort', 2181); -} sub getValue { my ($varname, $prefix) = @_; @@ -131,11 +127,12 @@ sub getCCSVar { return $default; } -sub getVar { +sub getServicesVar { my ($varname, $default, $warn) = @_; # print "GET var '$varname'\n"; my $cloud = getValue($varname, "services"); my $vespa = getValue($varname, "vespa_base"); + my $plain = $ENV{$varname}; if (defined($cloud) && defined($vespa)) { print STDERR "Found settings for both services.$varname and vespa_base.$varname, using settings from services\n"; } @@ -143,22 +140,32 @@ sub getVar { return $cloud; } elsif (defined($vespa)) { return $vespa; + } elsif (defined($plain)) { + return $plain; } elsif ($warn > 0) { print STDERR "No value found for 'services.$varname' or 'vespa_base.$varname'; using '$default'\n"; } return $default; } +sub getConfigServerPort { + my $port = getServicesVar('port_configserver_rpc', $default_configserver_port, 0); + return $port; +} + sub printConfigServerPort { - my $port = getVar('port_configserver_rpc', $default_configserver_port, 0); + my $port = getConfigServerPort(); print "$port\n"; } sub getConfigServers { my @ret; - my $addr = getVar('addr_configserver', $myHostname, 1); - my $port = getVar('port_configserver_rpc', $default_configserver_port, 0); + my $addr = $ENV{'VESPA_CONFIGSERVERS'}; + if (! defined($addr)) { + $addr = getServicesVar('addr_configserver', $myHostname, 1); + } + my $port = getConfigServerPort(); my $h; foreach $h (split(/,|\s+/, $addr)) { @@ -171,7 +178,12 @@ sub getConfigServers { return @ret; } +sub getZKPort { + my $zk_client_port = getCCSVar('zookeeper_clientPort', 2181); + return $zk_client_port; +} sub getZKString { + my $zk_client_port = getZKPort(); my $out; my $addr; foreach $addr (getConfigServers()) { @@ -188,8 +200,8 @@ sub printZKString { } sub printAllConfigSourcesWithPort { - my $cfport = getVar('port_configserver_rpc', $default_configserver_port, 0); - my $cpport = getVar('port_configproxy_rpc', $default_configproxy_port, 0); + my $cfport = getConfigServerPort(); + my $cpport = getServicesVar('port_configproxy_rpc', $default_configproxy_port, 0); my $addr = "localhost"; my $out = "tcp/${addr}:${cpport}"; foreach $addr (getConfigServers()) { @@ -279,39 +291,33 @@ if ( @ARGV == 0 ) { } if ( $ARGV[0] eq "-allconfigsources" ) { - vespa_base_env(); printAllConfigSourcesWithPort(); exit 0; } if ( $ARGV[0] eq "-configsources" ) { - vespa_base_env(); printConfigSources(); exit 0; } if ( $ARGV[0] eq "-confighttpsources" ) { $lookupInConfig = 1; - vespa_base_env(); printConfigHttpSources(); exit 0; } if ( $ARGV[0] eq "-zkstring" ) { - vespa_base_env(); printZKString(); exit 0; } if ( $ARGV[0] eq "-configserverport" ) { $lookupInConfig = 1; - vespa_base_env(); printConfigServerPort(); exit 0; } if ( $ARGV[0] eq "-zkclientport" ) { - vespa_base_env(); + my $zk_client_port = getZKPort(); print "$zk_client_port\n"; exit 0; } if ( $ARGV[0] eq "-isthisaconfigserver" ) { - vespa_base_env(); isThisAConfigServer(); exit 0; } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ConfigCurator.java b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ConfigCurator.java index faf25a16321..92d7589ea43 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ConfigCurator.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ConfigCurator.java @@ -392,7 +392,7 @@ public class ConfigCurator { catch (Exception e) { log.log(LogLevel.ERROR, "Unable to contact ZooKeeper on " + curator.connectionSpec() + ". Please verify for all configserver nodes that " + - "services.addr_configserver points to the correct configserver(s), " + + "VESPA_CONFIGSERVERS points to the correct configserver(s), " + "the same configserver(s) as in services.xml, and that they are started. " + "Check the log(s) for configserver errors. Aborting.", e); } diff --git a/configserver/src/main/sh/start-configserver b/configserver/src/main/sh/start-configserver index eed616cfe35..daad42f7895 100755 --- a/configserver/src/main/sh/start-configserver +++ b/configserver/src/main/sh/start-configserver @@ -87,7 +87,7 @@ fixddir ${VESPA_HOME}/var/zookeeper/version-2 ${VESPA_HOME}/libexec/vespa/vespa-config.pl -isthisaconfigserver 1>/dev/null if [ "$?" != "0" ] ; then - echo "Not able to start config server, host `hostname` is not part of 'services.addr_configserver'" + echo "Not able to start config server, host `hostname` is not part of 'VESPA_CONFIGSERVERS'" exit 1; fi diff --git a/container-accesslogging/src/main/java/com/yahoo/container/logging/LogFileHandler.java b/container-accesslogging/src/main/java/com/yahoo/container/logging/LogFileHandler.java index abbe346e6e9..52d26236ec7 100644 --- a/container-accesslogging/src/main/java/com/yahoo/container/logging/LogFileHandler.java +++ b/container-accesslogging/src/main/java/com/yahoo/container/logging/LogFileHandler.java @@ -266,13 +266,12 @@ public class LogFileHandler extends StreamHandler { } } - private void triggerCompression(String oldFileName) throws InterruptedException { + private void triggerCompression(String oldFileName) { try { Runtime r = Runtime.getRuntime(); Process p = r.exec(new String[] { "gzip", oldFileName }); // Detonator pattern: Think of all the fun we can have if gzip isn't what we // think it is, if it doesn't return, etc, etc - p.waitFor(); } catch (IOException e) { // little we can do... } diff --git a/container-dependency-versions/pom.xml b/container-dependency-versions/pom.xml index 68736c39623..4b824edbb21 100644 --- a/container-dependency-versions/pom.xml +++ b/container-dependency-versions/pom.xml @@ -439,7 +439,7 @@ <findbugs.version>1.3.9</findbugs.version> <guava.version>18.0</guava.version> <guice.version>3.0</guice.version> - <jetty.version>9.4.8.v20171121</jetty.version> + <jetty.version>9.4.9.v20180320</jetty.version> <scala.version>2.11.4</scala.version> <!-- When updating this, the scala.major-version in parent must also be updated! --> <slf4j.version>1.7.5</slf4j.version> diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/metric/MetricUpdater.java b/container-disc/src/main/java/com/yahoo/container/jdisc/metric/MetricUpdater.java index 3d026172c86..22b049c9ab7 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/metric/MetricUpdater.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/metric/MetricUpdater.java @@ -4,6 +4,7 @@ package com.yahoo.container.jdisc.metric; import com.google.inject.Inject; import com.yahoo.component.AbstractComponent; import com.yahoo.jdisc.Metric; +import com.yahoo.jdisc.statistics.ContainerWatchdogMetrics; import java.nio.file.DirectoryStream; import java.nio.file.Files; @@ -34,13 +35,13 @@ public class MetricUpdater extends AbstractComponent { private final Scheduler scheduler; @Inject - public MetricUpdater(Metric metric) { - this(new TimerScheduler(), metric); + public MetricUpdater(Metric metric, ContainerWatchdogMetrics containerWatchdogMetrics) { + this(new TimerScheduler(), metric, containerWatchdogMetrics); } - MetricUpdater(Scheduler scheduler, Metric metric) { + MetricUpdater(Scheduler scheduler, Metric metric, ContainerWatchdogMetrics containerWatchdogMetrics) { this.scheduler = scheduler; - scheduler.schedule(new UpdaterTask(metric), Duration.ofSeconds(10)); + scheduler.schedule(new UpdaterTask(metric, containerWatchdogMetrics), Duration.ofSeconds(10)); } @Override @@ -87,9 +88,11 @@ public class MetricUpdater extends AbstractComponent { private final Runtime runtime = Runtime.getRuntime(); private final Metric metric; + private final ContainerWatchdogMetrics containerWatchdogMetrics; - public UpdaterTask(Metric metric) { + public UpdaterTask(Metric metric, ContainerWatchdogMetrics containerWatchdogMetrics) { this.metric = metric; + this.containerWatchdogMetrics = containerWatchdogMetrics; } @SuppressWarnings("deprecation") @@ -106,6 +109,7 @@ public class MetricUpdater extends AbstractComponent { metric.set(TOTAL_MEMORY_BYTES, totalMemory, null); metric.set(MEMORY_MAPPINGS_COUNT, count_mappings(), null); metric.set(OPEN_FILE_DESCRIPTORS, count_open_files(), null); + containerWatchdogMetrics.emitMetrics(metric); } } diff --git a/container-disc/src/test/java/com/yahoo/container/jdisc/metric/MetricUpdaterTest.java b/container-disc/src/test/java/com/yahoo/container/jdisc/metric/MetricUpdaterTest.java index d94cea033f5..f10af7593a4 100644 --- a/container-disc/src/test/java/com/yahoo/container/jdisc/metric/MetricUpdaterTest.java +++ b/container-disc/src/test/java/com/yahoo/container/jdisc/metric/MetricUpdaterTest.java @@ -2,6 +2,7 @@ package com.yahoo.container.jdisc.metric; import com.yahoo.jdisc.Metric; +import com.yahoo.jdisc.statistics.ContainerWatchdogMetrics; import org.junit.Test; import java.time.Duration; @@ -20,7 +21,9 @@ public class MetricUpdaterTest { @Test public void metrics_are_updated_in_scheduler_cycle() throws InterruptedException { Metric metric = mock(Metric.class); - new MetricUpdater(new MockScheduler(), metric); + ContainerWatchdogMetrics containerWatchdogMetrics = mock(ContainerWatchdogMetrics.class); + new MetricUpdater(new MockScheduler(), metric, containerWatchdogMetrics); + verify(containerWatchdogMetrics, times(1)).emitMetrics(any()); verify(metric, times(8)).set(anyString(), any(), any()); } diff --git a/container-search/src/main/java/com/yahoo/fs4/QueryPacket.java b/container-search/src/main/java/com/yahoo/fs4/QueryPacket.java index 0709baa9be2..b8242ff5101 100644 --- a/container-search/src/main/java/com/yahoo/fs4/QueryPacket.java +++ b/container-search/src/main/java/com/yahoo/fs4/QueryPacket.java @@ -15,7 +15,6 @@ import com.yahoo.vespa.objects.BufferSerializer; import java.nio.ByteBuffer; import java.util.List; - /** * An "extended query" packet. This is the query packets used today, * they allow more flexible sets of parameters to be shipped with queries. @@ -26,12 +25,13 @@ import java.util.List; */ public class QueryPacket extends Packet { - final private Query query; + private final Query query; + private QueryPacketData queryPacketData; - int sessionOffset = 0; // Start of sessionKey ignore section for cache key - int sessionSize = 0; // Length of sessionKey ignore section for cache key - int ignoreableOffset = 0; // Start of (hits/offset/timestamp) ignore section for cache key - int ignoreableSize = 0; // Length of (hits/offset/timestamp) ignore section for cache key + private int sessionOffset = 0; // Start of sessionKey ignore section for cache key + private int sessionSize = 0; // Length of sessionKey ignore section for cache key + private int ignoreableOffset = 0; // Start of (hits/offset/timestamp) ignore section for cache key + private int ignoreableSize = 0; // Length of (hits/offset/timestamp) ignore section for cache key private QueryPacket(Query query) { this.query = query; @@ -80,6 +80,7 @@ public class QueryPacket extends Packet { private int getSessionKeySkipLength() { return (sessionSize > 0) ? sessionSize + 4 : 0; } + /** * Returns an opaque cache key for the query represented by this * (pre-serialized) packet. @@ -191,14 +192,14 @@ public class QueryPacket extends Packet { /** * feature bits, taken from searchlib/common/transport.h - **/ - static final int QF_PARSEDQUERY = 0x00000002; - static final int QF_RANKP = 0x00000004; - static final int QF_SORTSPEC = 0x00000080; - static final int QF_LOCATION = 0x00000800; - static final int QF_PROPERTIES = 0x00100000; - static final int QF_GROUPSPEC = 0x00400000; - static final int QF_SESSIONID = 0x00800000; + */ + private static final int QF_PARSEDQUERY = 0x00000002; + private static final int QF_RANKP = 0x00000004; + private static final int QF_SORTSPEC = 0x00000080; + private static final int QF_LOCATION = 0x00000800; + private static final int QF_PROPERTIES = 0x00100000; + private static final int QF_GROUPSPEC = 0x00400000; + private static final int QF_SESSIONID = 0x00800000; private int getFeatureInt(boolean sendSessionId) { int features = QF_PARSEDQUERY | QF_RANKP; // this bitmask means "parsed query" in query packet. @@ -215,27 +216,27 @@ public class QueryPacket extends Packet { /** * query flag bits, taken from searchlib/common/transport.h - **/ - static final int QFLAG_EXTENDED_COVERAGE = 0x00000001; - static final int QFLAG_COVERAGE_NODES = 0x00000002; - static final int QFLAG_ESTIMATE = 0x00000080; - static final int QFLAG_DROP_SORTDATA = 0x00004000; - static final int QFLAG_NO_RESULTCACHE = 0x00010000; - static final int QFLAG_DUMP_FEATURES = 0x00040000; + */ + private static final int QFLAG_EXTENDED_COVERAGE = 0x00000001; + private static final int QFLAG_COVERAGE_NODES = 0x00000002; + private static final int QFLAG_ESTIMATE = 0x00000080; + private static final int QFLAG_DROP_SORTDATA = 0x00004000; + private static final int QFLAG_NO_RESULTCACHE = 0x00010000; + private static final int QFLAG_DUMP_FEATURES = 0x00040000; private int getFlagInt() { int flags = getQueryFlags(query); queryPacketData.setQueryFlags(flags); - /** + /* * QFLAG_DROP_SORTDATA * SORTDATA is a mangling of data from the attribute vectors * which were used in the search which is byte comparable in * such a way the comparing SORTDATA for two different hits * will reproduce the order in which the data were returned when - * using sortspec. For now we simple drop these, but if they - * should be necessary at later date, QueryResultPacket must be - * updated to be able to parse result packets correctly. + * using sortspec. For now we simply drop these. If they + * become necessar, QueryResultPacket must be + * updated to be able to read the sort data. */ flags |= QFLAG_DROP_SORTDATA; return flags; @@ -264,13 +265,12 @@ public class QueryPacket extends Packet { * creating a summary request. * * @return wrapper object suitable for creating a summary fetch packet - * @throws IllegalStateException - * if no wrapper has been generated + * @throws IllegalStateException if no wrapper has been generated */ public QueryPacketData getQueryPacketData() { - if (queryPacketData == null) { + if (queryPacketData == null) throw new IllegalStateException("Trying to fetch a hit tag without having encoded the packet first."); - } return queryPacketData; } + } diff --git a/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java b/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java index 2709755da5d..258f0dec9ff 100644 --- a/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/cluster/ClusterSearcher.java @@ -13,6 +13,7 @@ import com.yahoo.container.handler.VipStatus; import com.yahoo.fs4.mplex.Backend; import com.yahoo.container.search.LegacyEmulationConfig; import com.yahoo.net.HostName; +import com.yahoo.prelude.fastsearch.DocsumDefinitionSet; import com.yahoo.search.dispatch.Dispatcher; import com.yahoo.prelude.fastsearch.FS4ResourcePool; import com.yahoo.prelude.IndexFacts; diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/CacheControl.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/CacheControl.java index c1c818848ad..b7e16b6c082 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/CacheControl.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/CacheControl.java @@ -12,7 +12,7 @@ import com.yahoo.processing.request.CompoundName; /** * The cache control logic for FastSearcher * - * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a> + * @author Steinar Knutsen */ public class CacheControl { diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumDefinition.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumDefinition.java index 4fd0f884903..d50006fb82c 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumDefinition.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumDefinition.java @@ -21,20 +21,33 @@ import java.util.Map; */ public class DocsumDefinition { - private String name; - private final List<DocsumField> fields; + private final String name; + private final ImmutableList<DocsumField> fields; /** True if this contains dynamic fields */ - private boolean dynamic = false; + private final boolean dynamic; // Mapping between field names and their index in this.fields - private final Map<String,Integer> fieldNameToIndex; + private final ImmutableMap<String, Integer> fieldNameToIndex; + + public DocsumDefinition(String name, List<DocsumField> fields) { + this.name = name; + this.dynamic = false; + this.fields = ImmutableList.copyOf(fields); + ImmutableMap.Builder<String, Integer> fieldNameToIndexBuilder = new ImmutableMap.Builder<>(); + int i = 0; + for (DocsumField field : fields) + fieldNameToIndexBuilder.put(field.name, i++); + this.fieldNameToIndex = fieldNameToIndexBuilder.build(); + } + // TODO: Remove LegacyEmulationConfig (the config, not just the usage) on Vespa 7 DocsumDefinition(DocumentdbInfoConfig.Documentdb.Summaryclass config, LegacyEmulationConfig emulConfig) { this.name = config.name(); - List<DocsumField> fieldsBuilder = new ArrayList<>(); - Map<String,Integer> fieldNameToIndexBuilder = new HashMap<>(); + List<DocsumField> fieldsBuilder = new ArrayList<>(); + Map<String, Integer> fieldNameToIndexBuilder = new HashMap<>(); + boolean dynamic = false; for (DocumentdbInfoConfig.Documentdb.Summaryclass.Fields field : config.fields()) { // no, don't switch the order of the two next lines :) fieldNameToIndexBuilder.put(field.name(), fieldsBuilder.size()); @@ -42,6 +55,7 @@ public class DocsumDefinition { if (field.dynamic()) dynamic = true; } + this.dynamic = dynamic; fields = ImmutableList.copyOf(fieldsBuilder); fieldNameToIndex = ImmutableMap.copyOf(fieldNameToIndexBuilder); } diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumDefinitionSet.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumDefinitionSet.java index 8d882adeb02..3ecc01a5e8e 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumDefinitionSet.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumDefinitionSet.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.prelude.fastsearch; +import com.google.common.collect.ImmutableMap; import com.yahoo.slime.BinaryFormat; import com.yahoo.data.access.Inspector; import com.yahoo.slime.Slime; @@ -10,9 +11,12 @@ import com.yahoo.container.search.LegacyEmulationConfig; import java.nio.ByteBuffer; import java.nio.ByteOrder; +import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.Map; import java.util.logging.Logger; +import java.util.stream.Collectors; import static com.yahoo.data.access.Type.OBJECT; @@ -27,19 +31,28 @@ public final class DocsumDefinitionSet { public static final int SLIME_MAGIC_ID = 0x55555555; private final static Logger log = Logger.getLogger(DocsumDefinitionSet.class.getName()); - private final HashMap<String, DocsumDefinition> definitionsByName = new HashMap<>(); + private final Map<String, DocsumDefinition> definitionsByName; private final LegacyEmulationConfig emulationConfig; public DocsumDefinitionSet(DocumentdbInfoConfig.Documentdb config) { - this.emulationConfig = new LegacyEmulationConfig(new LegacyEmulationConfig.Builder()); - configure(config); + this(config, new LegacyEmulationConfig(new LegacyEmulationConfig.Builder())); } public DocsumDefinitionSet(DocumentdbInfoConfig.Documentdb config, LegacyEmulationConfig emulConfig) { + this(toDocsums(config, emulConfig), emulConfig); + } + + public DocsumDefinitionSet(Collection<DocsumDefinition> docsumDefinitions) { + this(docsumDefinitions, new LegacyEmulationConfig(new LegacyEmulationConfig.Builder())); + } + + public DocsumDefinitionSet(Collection<DocsumDefinition> docsumDefinitions, LegacyEmulationConfig emulConfig) { + this.definitionsByName = ImmutableMap.copyOf(docsumDefinitions.stream().collect(Collectors.toMap(DocsumDefinition::getName, p -> p))); this.emulationConfig = emulConfig; - configure(config); } + LegacyEmulationConfig legacyEmulationConfig() { return emulationConfig; } + /** * Returns a docsum definition by name, or null if not found * @@ -106,14 +119,13 @@ public final class DocsumDefinitionSet { return definitionsByName.size(); } - private void configure(DocumentdbInfoConfig.Documentdb config) { - for (int i = 0; i < config.summaryclass().size(); ++i) { - DocumentdbInfoConfig.Documentdb.Summaryclass sc = config.summaryclass(i); - DocsumDefinition docSumDef = new DocsumDefinition(sc, emulationConfig); - definitionsByName.put(sc.name(), docSumDef); - } - if (definitionsByName.size() == 0) { + private static Collection<DocsumDefinition> toDocsums(DocumentdbInfoConfig.Documentdb config, LegacyEmulationConfig emulConfig) { + Collection<DocsumDefinition> docsums = new ArrayList<>(); + for (int i = 0; i < config.summaryclass().size(); ++i) + docsums.add(new DocsumDefinition(config.summaryclass(i), emulConfig)); + if (docsums.isEmpty()) log.warning("No summary classes found in DocumentdbInfoConfig.Documentdb"); - } + return docsums; } + } diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumField.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumField.java index 1e44a8fa64d..d5e4eb75931 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumField.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumField.java @@ -73,8 +73,8 @@ public abstract class DocsumField { this.name = name; } - /* for unit test only */ - static DocsumField create(String name, String typename) { + /* For unit test only */ + public static DocsumField create(String name, String typename) { return create(name, typename, new LegacyEmulationConfig(new LegacyEmulationConfig.Builder())); } @@ -114,7 +114,7 @@ public abstract class DocsumField { /** * Convert a generic value into an object of the appropriate type * for this field. - **/ + */ public abstract Object convert(Inspector value); } diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocumentDatabase.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocumentDatabase.java index 3366f92384a..0ae0983a1ae 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocumentDatabase.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocumentDatabase.java @@ -4,10 +4,13 @@ package com.yahoo.prelude.fastsearch; import com.google.common.collect.ImmutableMap; import com.yahoo.container.search.LegacyEmulationConfig; +import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; /** * Representation of a back-end document database. @@ -24,12 +27,16 @@ public class DocumentDatabase { private final String name; private final DocsumDefinitionSet docsumDefSet; - private final Map<String, RankProfile> rankProfiles; + private final ImmutableMap<String, RankProfile> rankProfiles; public DocumentDatabase(DocumentdbInfoConfig.Documentdb documentDb, LegacyEmulationConfig emulConfig) { - this.name = documentDb.name(); - this.docsumDefSet = new DocsumDefinitionSet(documentDb, emulConfig); - this.rankProfiles = ImmutableMap.copyOf(toRankProfiles(documentDb.rankprofile())); + this(documentDb.name(), new DocsumDefinitionSet(documentDb, emulConfig), toRankProfiles(documentDb.rankprofile())); + } + + public DocumentDatabase(String name, DocsumDefinitionSet docsumDefinitionSet, Collection<RankProfile> rankProfiles) { + this.name = name; + this.docsumDefSet = docsumDefinitionSet; + this.rankProfiles = ImmutableMap.copyOf(rankProfiles.stream().collect(Collectors.toMap(RankProfile::getName, p -> p))); } public String getName() { @@ -43,11 +50,14 @@ public class DocumentDatabase { /** Returns an unmodifiable map of all the rank profiles in this indexed by rank profile name */ public Map<String, RankProfile> rankProfiles() { return rankProfiles; } - private Map<String, RankProfile> toRankProfiles(List<DocumentdbInfoConfig.Documentdb.Rankprofile> rankProfileConfigList) { - Map<String, RankProfile> rankProfiles = new HashMap<>(); - for (DocumentdbInfoConfig.Documentdb.Rankprofile c : rankProfileConfigList) { - rankProfiles.put(c.name(), new RankProfile(c.name(), c.hasSummaryFeatures(), c.hasRankFeatures())); - } + private static ImmutableMap<String, RankProfile> toMap(Collection<RankProfile> rankProfiles) { + return ImmutableMap.copyOf(rankProfiles.stream().collect(Collectors.toMap(RankProfile::getName, p -> p))); + } + + private static Collection<RankProfile> toRankProfiles(Collection<DocumentdbInfoConfig.Documentdb.Rankprofile> rankProfileConfigList) { + List<RankProfile> rankProfiles = new ArrayList<>(); + for (DocumentdbInfoConfig.Documentdb.Rankprofile c : rankProfileConfigList) + rankProfiles.add(new RankProfile(c.name(), c.hasSummaryFeatures(), c.hasRankFeatures())); return rankProfiles; } diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java index aab8aae6025..b3eaee8698a 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java @@ -74,9 +74,10 @@ public class FastHit extends Hit { setPartId(0, 0); } + @Override public String toString() { return super.toString() + " [fasthit, globalid: " + globalId + ", partId: " - + partId + ", distributionkey: " + distributionKey + "]"; + + partId + ", distributionkey: " + distributionKey + "]"; } public static String asHexString(GlobalId gid) { @@ -267,7 +268,8 @@ public class FastHit extends Hit { this.distributionKey = distributionKey; } - void addSummary(DocsumDefinition docsumDef, Inspector value) { + /** For internal use */ + public void addSummary(DocsumDefinition docsumDef, Inspector value) { reserve(docsumDef.getFieldCount()); for (DocsumField field : docsumDef.getFields()) { String fieldName = field.getName(); @@ -290,10 +292,8 @@ public class FastHit extends Hit { * easier. This is not a method to be used for efficiency, as it causes * object allocations. * - * @param fieldName - * the name of the field to insert undecoded UTF-8 into - * @param value - * an array of valid UTF-8 data + * @param fieldName the name of the field to insert undecoded UTF-8 into + * @param value an array of valid UTF-8 data */ @Beta public void setLazyStringField(String fieldName, byte[] value) { diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java index 7d822fd603b..e5f255f18c0 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastSearcher.java @@ -4,6 +4,7 @@ package com.yahoo.prelude.fastsearch; import java.util.Optional; import com.yahoo.compress.CompressionType; +import com.yahoo.container.search.LegacyEmulationConfig; import com.yahoo.fs4.BasicPacket; import com.yahoo.fs4.ChannelTimeoutException; import com.yahoo.fs4.GetDocSumsPacket; @@ -15,7 +16,6 @@ import com.yahoo.fs4.QueryResultPacket; import com.yahoo.fs4.mplex.Backend; import com.yahoo.fs4.mplex.FS4Channel; import com.yahoo.fs4.mplex.InvalidChannelException; -import com.yahoo.net.HostName; import com.yahoo.prelude.Ping; import com.yahoo.prelude.Pong; import com.yahoo.prelude.querytransform.QueryRewrite; @@ -56,6 +56,9 @@ public class FastSearcher extends VespaBackEndSearcher { /** If this is turned on this will make search queries directly to the local search node when possible */ private final static CompoundName dispatchDirect = new CompoundName("dispatch.direct"); + /** Unless turned off this will fill summaries by dispatching directly to search nodes over RPC when possible */ + private final static CompoundName dispatchSummaries = new CompoundName("dispatch.summaries"); + /** The compression method which will be used with rpc dispatch. "lz4" (default) and "none" is supported. */ private final static CompoundName dispatchCompression = new CompoundName("dispatch.compression"); @@ -231,6 +234,7 @@ public class FastSearcher extends VespaBackEndSearcher { /** * Only used to fill the sddocname field when using direct dispatching as that is normally done in VespaBackEndSearcher.decodeSummary + * * @param result The result */ private void fillSDDocName(Result result) { @@ -255,11 +259,15 @@ public class FastSearcher extends VespaBackEndSearcher { Query query = result.getQuery(); traceQuery(getName(), "fill", query, query.getOffset(), query.getHits(), 2, quotedSummaryClass(summaryClass)); - if (wantsRPCSummaryFill(query)) { + if (query.properties().getBoolean(dispatchSummaries, true) + && ! summaryNeedsQuery(query) + && ! cacheControl.useCache(query) + && ! legacyEmulationConfigIsSet(getDocumentDatabase(query))) { + CompressionType compression = CompressionType.valueOf(query.properties().getString(dispatchCompression, "LZ4").toUpperCase()); fillSDDocName(result); - dispatcher.fill(result, summaryClass, compression); + dispatcher.fill(result, summaryClass, getDocumentDatabase(query), compression); return; } @@ -349,6 +357,14 @@ public class FastSearcher extends VespaBackEndSearcher { } } + private boolean legacyEmulationConfigIsSet(DocumentDatabase db) { + LegacyEmulationConfig config = db.getDocsumDefinitionSet().legacyEmulationConfig(); + if (config.forceFillEmptyFields()) return true; + if (config.stringBackedFeatureData()) return true; + if (config.stringBackedStructuredData()) return true; + return false; + } + private static @NonNull Optional<String> quotedSummaryClass(String summaryClass) { return Optional.of(summaryClass == null ? "[null]" : quote(summaryClass)); } @@ -465,14 +481,9 @@ public class FastSearcher extends VespaBackEndSearcher { } boolean couldSend = channel.sendPacket(docsumsPacket); - if (isLoggingFine()) - getLogger().finest("Sent " + docsumsPacket + " on " + channel); if ( ! couldSend) throw new IOException("Could not successfully send GetDocSumsPacket."); receivedPackets = channel.receivePackets(result.getQuery().getTimeLeft(), docsumsPacket.getNumDocsums() + 1); - if (isLoggingFine()) - getLogger().finest("got " + receivedPackets.length + "docsumPackets"); - return convertBasicPackets(receivedPackets); } diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FloatField.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FloatField.java index 6be5a27b49b..6c73167b162 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FloatField.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FloatField.java @@ -9,9 +9,10 @@ import com.yahoo.data.access.Inspector; /** - * @author <a href="mailto:mathiasm@yahoo-inc.com">Mathias M\u00f8lster Lidal</a> + * @author Mathias Mølster Lidal */ public class FloatField extends DocsumField { + static final double EMPTY_VALUE = Float.NaN; public FloatField(String name) { @@ -26,10 +27,12 @@ public class FloatField extends DocsumField { } } + @Override public Object decode(ByteBuffer b) { return convert(b.getFloat()); } + @Override public Object decode(ByteBuffer b, FastHit hit) { Object field = decode(b); hit.setField(name, field); @@ -46,4 +49,5 @@ public class FloatField extends DocsumField { public Object convert(Inspector value) { return convert((float)value.asDouble(EMPTY_VALUE)); } + } diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/IntegerField.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/IntegerField.java index fe44597b7d7..eef6fc73294 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/IntegerField.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/IntegerField.java @@ -12,9 +12,10 @@ import com.yahoo.search.result.NanNumber; import com.yahoo.data.access.Inspector; /** - * @author <a href="mailto:borud@yahoo-inc.com">Bj\u00f8rn Borud</a> + * @author Bjørn Borud */ public class IntegerField extends DocsumField { + static final int EMPTY_VALUE = Integer.MIN_VALUE; public IntegerField(String name) { @@ -25,20 +26,23 @@ public class IntegerField extends DocsumField { if (value == EMPTY_VALUE) { return NanNumber.NaN; } else { - return Integer.valueOf(value); + return value; } } + @Override public Object decode(ByteBuffer b) { return convert(b.getInt()); } + @Override public Object decode(ByteBuffer b, FastHit hit) { Object field = decode(b); hit.setField(name, field); return field; } + @Override public String toString() { return "field " + getName() + " type int"; } @@ -53,4 +57,5 @@ public class IntegerField extends DocsumField { public Object convert(Inspector value) { return convert((int)value.asLong(EMPTY_VALUE)); } + } diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/LongdataField.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/LongdataField.java index 08cd4ddff35..9d22168485c 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/LongdataField.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/LongdataField.java @@ -86,4 +86,5 @@ public class LongdataField extends DocsumField implements VariableLengthField { public Object convert(Inspector value) { return convert(value.asData(Value.empty().asData())); } + } diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java index eac1579a821..e5c9f048e53 100644 --- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java +++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/VespaBackEndSearcher.java @@ -55,8 +55,6 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { private static final CompoundName grouping=new CompoundName("grouping"); private static final CompoundName combinerows=new CompoundName("combinerows"); - /** If this is turned on this will fill summaries by dispatching directly to search nodes over RPC */ - private final static CompoundName dispatchSummaries = new CompoundName("dispatch.summaries"); protected static final CompoundName PACKET_COMPRESSION_LIMIT = new CompoundName("packetcompressionlimit"); protected static final CompoundName PACKET_COMPRESSION_TYPE = new CompoundName("packetcompressiontype"); @@ -110,10 +108,6 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { protected abstract void doPartialFill(Result result, String summaryClass); - protected static boolean wantsRPCSummaryFill(Query query) { - return query.properties().getBoolean(dispatchSummaries); - } - /** * Returns whether we need to send the query when fetching summaries. * This is necessary if the query requests summary features or dynamic snippeting @@ -216,12 +210,6 @@ public abstract class VespaBackEndSearcher extends PingableSearcher { return new Result(query, ErrorMessage.createNullQuery(query.getHttpRequest().getUri().toString())); } - if (wantsRPCSummaryFill(query) && summaryNeedsQuery(query)) { - return new Result(query, ErrorMessage.createInvalidQueryParameter( - "When using dispatch.summaries and your summary/rankprofile require the query, " + - " you need to enable ranking.queryCache.")); - } - QueryRewrite.optimizeByRestrict(query); QueryRewrite.optimizeAndNot(query); QueryRewrite.collapseSingleComposites(query); diff --git a/container-search/src/main/java/com/yahoo/prelude/hitfield/FieldPart.java b/container-search/src/main/java/com/yahoo/prelude/hitfield/FieldPart.java index df8f6e92d57..6ca5feb610f 100644 --- a/container-search/src/main/java/com/yahoo/prelude/hitfield/FieldPart.java +++ b/container-search/src/main/java/com/yahoo/prelude/hitfield/FieldPart.java @@ -4,11 +4,13 @@ package com.yahoo.prelude.hitfield; /** * Represents an element of a hit property * - * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a> + * @author Steinar Knutsen */ public interface FieldPart { - public abstract boolean isFinal(); - public abstract boolean isToken(); - public abstract String getContent(); - public abstract String toString(); + + boolean isFinal(); + boolean isToken(); + String getContent(); + String toString(); + } diff --git a/container-search/src/main/java/com/yahoo/prelude/hitfield/HitField.java b/container-search/src/main/java/com/yahoo/prelude/hitfield/HitField.java index 770ac24dcab..79d871d8c74 100644 --- a/container-search/src/main/java/com/yahoo/prelude/hitfield/HitField.java +++ b/container-search/src/main/java/com/yahoo/prelude/hitfield/HitField.java @@ -10,11 +10,10 @@ import com.yahoo.prelude.searcher.JuniperSearcher; import com.yahoo.text.XML; /** - * Represents a Field in a Hit. The original raw content and the field - * name cannot be modified. But the tokenized version can be retrieved - * and set. + * Represents a tokenized string field in a Hit. The original raw content and the field + * name cannot be modified. But the tokenized version can be retrieved and set. * - * @author <a href="mailto:larschr@yahoo-inc.com">Lars Christian Jensen</a> + * @author Lars Christian Jensen */ public class HitField { @@ -345,10 +344,7 @@ public class HitField { return xml.toString(); } - /** - * @return the content of this field, using the arguments as bolding - * tags, as an XML string - */ + /** Returns the content of this field, using the arguments as bolding tags, as an XML string */ public String quotedContent(String boldOpenTag, String boldCloseTag, String separatorTag, diff --git a/container-search/src/main/java/com/yahoo/prelude/hitfield/ImmutableFieldPart.java b/container-search/src/main/java/com/yahoo/prelude/hitfield/ImmutableFieldPart.java index fa0ca62405f..268e1b53459 100644 --- a/container-search/src/main/java/com/yahoo/prelude/hitfield/ImmutableFieldPart.java +++ b/container-search/src/main/java/com/yahoo/prelude/hitfield/ImmutableFieldPart.java @@ -2,12 +2,12 @@ package com.yahoo.prelude.hitfield; /** - * Represents an element of a hit property which is a possibly - * mutable string element + * Represents an element of a hit property which is an immutable string element * - * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a> + * @author Steinar Knutsen */ public class ImmutableFieldPart implements FieldPart { + private final String content; private final String initContent; // Whether this element represents a (part of) a token or a @@ -15,10 +15,12 @@ public class ImmutableFieldPart implements FieldPart { // parts should inherit this state from the object they were // split from. private boolean tokenOrDelimiter; + public ImmutableFieldPart(String initContent, boolean tokenOrDelimiter) { this(initContent, initContent, tokenOrDelimiter); } + public ImmutableFieldPart(String initContent, String content, boolean tokenOrDelimiter) { @@ -27,9 +29,17 @@ public class ImmutableFieldPart implements FieldPart { this.content = content; this.tokenOrDelimiter = tokenOrDelimiter; } + + @Override public boolean isFinal() { return true; } + @Override public boolean isToken() { return tokenOrDelimiter; } + @Override public String getContent() { return content; } + public String getInitContent() { return initContent; } + + @Override public String toString() { return content; } + } diff --git a/container-search/src/main/java/com/yahoo/prelude/hitfield/JSONString.java b/container-search/src/main/java/com/yahoo/prelude/hitfield/JSONString.java index bdd7cf04c3e..06db012309e 100644 --- a/container-search/src/main/java/com/yahoo/prelude/hitfield/JSONString.java +++ b/container-search/src/main/java/com/yahoo/prelude/hitfield/JSONString.java @@ -19,7 +19,7 @@ import java.util.Iterator; /** * A JSON wrapper. Contains XML-style rendering of a JSON structure. * - * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a> + * @author Steinar Knutsen */ public class JSONString implements Inspectable { diff --git a/container-search/src/main/java/com/yahoo/prelude/hitfield/MarkupFieldPart.java b/container-search/src/main/java/com/yahoo/prelude/hitfield/MarkupFieldPart.java index 23ca8272851..17def6771ae 100644 --- a/container-search/src/main/java/com/yahoo/prelude/hitfield/MarkupFieldPart.java +++ b/container-search/src/main/java/com/yahoo/prelude/hitfield/MarkupFieldPart.java @@ -4,19 +4,31 @@ package com.yahoo.prelude.hitfield; /** * Represents an element of a hit property which is markup, not content. * - * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a> + * @author Steinar Knutsen */ public class MarkupFieldPart implements FieldPart { + private String content; + public MarkupFieldPart(String content) { this.content = content; } + + @Override public boolean isFinal() { return true; } + // Markup is never part of tokens as such + @Override public boolean isToken() { return false; } + public void setContent(String content) { this.content = content; } + + @Override public String getContent() { return content; } + + @Override public String toString() { return content; } + } diff --git a/container-search/src/main/java/com/yahoo/prelude/hitfield/RawData.java b/container-search/src/main/java/com/yahoo/prelude/hitfield/RawData.java index dbbbca63d43..a0c9b10c519 100644 --- a/container-search/src/main/java/com/yahoo/prelude/hitfield/RawData.java +++ b/container-search/src/main/java/com/yahoo/prelude/hitfield/RawData.java @@ -6,8 +6,8 @@ package com.yahoo.prelude.hitfield; * * @author arnej27959 */ -public final class RawData -{ +public final class RawData { + private byte[] content; /** @@ -52,4 +52,5 @@ public final class RawData } return buf.toString(); } + } diff --git a/container-search/src/main/java/com/yahoo/prelude/hitfield/StringFieldPart.java b/container-search/src/main/java/com/yahoo/prelude/hitfield/StringFieldPart.java index d7607fb7100..58018305fef 100644 --- a/container-search/src/main/java/com/yahoo/prelude/hitfield/StringFieldPart.java +++ b/container-search/src/main/java/com/yahoo/prelude/hitfield/StringFieldPart.java @@ -5,9 +5,10 @@ package com.yahoo.prelude.hitfield; * Represents an element of a hit property which is a possibly * mutable string element * - * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a> + * @author Steinar Knutsen */ public class StringFieldPart implements FieldPart { + private String content; private final String initContent; // Whether this element represents a (part of) a token or a @@ -15,17 +16,28 @@ public class StringFieldPart implements FieldPart { // parts should inherit this state from the object they were // split from. private boolean tokenOrDelimiter; + public StringFieldPart(String content, boolean tokenOrDelimiter) { this.content = content; initContent = content; this.tokenOrDelimiter = tokenOrDelimiter; } + + @Override public boolean isFinal() { return false; } + + @Override public boolean isToken() { return tokenOrDelimiter; } + + @Override public String getContent() { return content; } + public void setContent(String content) { this.content = content; } public String getInitContent() { return initContent; } + + @Override public String toString() { return content; } + } diff --git a/container-search/src/main/java/com/yahoo/prelude/hitfield/XmlRenderer.java b/container-search/src/main/java/com/yahoo/prelude/hitfield/XmlRenderer.java index 8bafbfd6ab5..99c5daa05b8 100644 --- a/container-search/src/main/java/com/yahoo/prelude/hitfield/XmlRenderer.java +++ b/container-search/src/main/java/com/yahoo/prelude/hitfield/XmlRenderer.java @@ -1,21 +1,14 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.prelude.hitfield; -import com.yahoo.text.Utf8; import com.yahoo.text.XML; import com.yahoo.data.access.Inspector; -import com.yahoo.data.access.Inspectable; import com.yahoo.data.access.Type; -import com.yahoo.data.access.simple.Value; -import com.yahoo.data.access.slime.SlimeAdapter; -import java.nio.charset.StandardCharsets; - -import java.util.Iterator; import java.util.Map; /** * Utility class for converting accessible data into the historical "prelude" xml format. - **/ + */ public class XmlRenderer { public static StringBuilder render(StringBuilder target, Inspector value) { diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java index 1b775d2c46f..1b6ba5cc085 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/Dispatcher.java @@ -9,6 +9,7 @@ import com.yahoo.compress.CompressionType; import com.yahoo.compress.Compressor; import com.yahoo.container.handler.VipStatus; import com.yahoo.container.protect.Error; +import com.yahoo.prelude.fastsearch.DocumentDatabase; import com.yahoo.slime.ArrayTraverser; import com.yahoo.data.access.slime.SlimeAdapter; import com.yahoo.prelude.fastsearch.FS4ResourcePool; @@ -22,12 +23,9 @@ import com.yahoo.search.result.Hit; import com.yahoo.data.access.Inspector; import com.yahoo.slime.BinaryFormat; import com.yahoo.slime.Cursor; -import com.yahoo.slime.JsonFormat; import com.yahoo.slime.Slime; import com.yahoo.vespa.config.search.DispatchConfig; -import java.io.ByteArrayOutputStream; -import java.io.IOException; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -38,12 +36,15 @@ import java.util.logging.Level; import java.util.logging.Logger; /** - * A dispatcher communicates with search nodes to (in the future) perform queries and (now) fill hits. + * A dispatcher communicates with search nodes to perform queries and fill hits. + * + * This is currently not functionally complete: Queries can only be dispatched to a single node, + * and summaries can only be requested when they do not need the query. + * * This class is multithread safe. * * @author bratseth */ -@Beta public class Dispatcher extends AbstractComponent { private final static Logger log = Logger.getLogger(Dispatcher.class.getName()); @@ -82,15 +83,21 @@ public class Dispatcher extends AbstractComponent { public SearchCluster searchCluster() { return searchCluster; } /** Fills the given summary class by sending RPC requests to the right search nodes */ - public void fill(Result result, String summaryClass, CompressionType compression) { + public void fill(Result result, String summaryClass, DocumentDatabase documentDb, CompressionType compression) { try { ListMap<Integer, FastHit> hitsByNode = hitsByNode(result); + if (result.getQuery().getTraceLevel() >=3) + result.getQuery().trace("Sending " + hitsByNode.size() + " summary fetch RPC requests", 3); + GetDocsumsResponseReceiver responseReceiver = new GetDocsumsResponseReceiver(hitsByNode.size(), compressor, result); for (Map.Entry<Integer, List<FastHit>> nodeHits : hitsByNode.entrySet()) { sendGetDocsumsRequest(nodeHits.getKey(), nodeHits.getValue(), summaryClass, compression, result, responseReceiver); } - responseReceiver.processResponses(result.getQuery(), summaryClass); + responseReceiver.processResponses(result.getQuery(), summaryClass, documentDb); + result.hits().setFilled(summaryClass); + result.hits().setSorted(false); + result.analyzeHits(); } catch (TimeoutException e) { result.hits().addError(ErrorMessage.createTimeout("Summary data is incomplete: " + e.getMessage())); @@ -192,7 +199,7 @@ public class Dispatcher extends AbstractComponent { * Call this from the dispatcher thread to initiate and complete processing of responses. * This will block until all responses are available and processed, or to timeout. */ - public void processResponses(Query query, String summaryClass) throws TimeoutException { + public void processResponses(Query query, String summaryClass, DocumentDatabase documentDb) throws TimeoutException { try { int skippedHits = 0; while (outstandingResponses > 0) { @@ -203,11 +210,12 @@ public class Dispatcher extends AbstractComponent { Client.GetDocsumsResponseOrError response = responses.poll(timeLeftMs, TimeUnit.MILLISECONDS); if (response == null) throwTimeout(); - skippedHits += processResponse(response); + skippedHits += processResponse(response, summaryClass, documentDb); outstandingResponses--; } if (skippedHits != 0) { - result.hits().addError(com.yahoo.search.result.ErrorMessage.createEmptyDocsums("Missing hit summary data for summary " + summaryClass + " for " + skippedHits + " hits")); + result.hits().addError(com.yahoo.search.result.ErrorMessage.createEmptyDocsums("Missing hit summary data for summary " + + summaryClass + " for " + skippedHits + " hits")); } } catch (InterruptedException e) { @@ -215,7 +223,9 @@ public class Dispatcher extends AbstractComponent { } } - private int processResponse(Client.GetDocsumsResponseOrError responseOrError) { + private int processResponse(Client.GetDocsumsResponseOrError responseOrError, + String summaryClass, + DocumentDatabase documentDb) { if (responseOrError.error().isPresent()) { if (hasReportedError) return 0; String error = responseOrError.error().get(); @@ -226,7 +236,7 @@ public class Dispatcher extends AbstractComponent { Client.GetDocsumsResponse response = responseOrError.response().get(); CompressionType compression = CompressionType.valueOf(response.compression()); byte[] slimeBytes = compressor.decompress(response.compressedSlimeBytes(), compression, response.uncompressedSize()); - return fill(response.hitsContext(), slimeBytes); + return fill(response.hitsContext(), summaryClass, documentDb, slimeBytes); } return 0; } @@ -241,7 +251,7 @@ public class Dispatcher extends AbstractComponent { }); } - private int fill(List<FastHit> hits, byte[] slimeBytes) { + private int fill(List<FastHit> hits, String summaryClass, DocumentDatabase documentDb, byte[] slimeBytes) { com.yahoo.slime.Inspector root = BinaryFormat.decode(slimeBytes).get(); com.yahoo.slime.Inspector errors = root.field("errors"); boolean hasErrors = errors.valid() && (errors.entries() > 0); @@ -256,7 +266,7 @@ public class Dispatcher extends AbstractComponent { for (int i = 0; i < hits.size(); i++) { Inspector summary = summaries.entry(i).field("docsum"); if (summary.fieldCount() != 0) { - fill(hits.get(i), summary); + hits.get(i).addSummary(documentDb.getDocsumDefinitionSet().getDocsumDefinition(summaryClass), summary); } else { skippedHits++; } @@ -264,27 +274,6 @@ public class Dispatcher extends AbstractComponent { return skippedHits; } - private void fill(FastHit hit, Inspector summary) { - hit.reserve(summary.fieldCount()); - summary.traverse((String name, Inspector value) -> { - hit.setField(name, nativeTypeOf(value)); - }); - } - - private Object nativeTypeOf(Inspector inspector) { - switch (inspector.type()) { - case ARRAY: return inspector; - case OBJECT: return inspector; - case BOOL: return inspector.asBool(); - case DATA: return inspector.asData(); - case DOUBLE: return inspector.asDouble(); - case LONG: return inspector.asLong(); - case STRING: return inspector.asString(); // TODO: Keep as utf8 - case EMPTY : return null; - default: throw new IllegalArgumentException("Unexpected Slime type " + inspector.type()); - } - } - } } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/SearchCluster.java b/container-search/src/main/java/com/yahoo/search/dispatch/SearchCluster.java index 60f2691aa69..9b2a24cd01f 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/SearchCluster.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/SearchCluster.java @@ -41,7 +41,6 @@ import java.util.stream.Collectors; * * @author bratseth */ -@Beta public class SearchCluster implements NodeManager<SearchCluster.Node> { private static final Logger log = Logger.getLogger(SearchCluster.class.getName()); diff --git a/container-search/src/main/java/com/yahoo/search/grouping/request/LongValue.java b/container-search/src/main/java/com/yahoo/search/grouping/request/LongValue.java index aa2de1a1298..2f5d833a5be 100644 --- a/container-search/src/main/java/com/yahoo/search/grouping/request/LongValue.java +++ b/container-search/src/main/java/com/yahoo/search/grouping/request/LongValue.java @@ -4,16 +4,17 @@ package com.yahoo.search.grouping.request; /** * This class represents a constant {@link Long} value in a {@link GroupingExpression}. * - * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen</a> + * @author Simon Thoresen */ public class LongValue extends ConstantValue<Long> { /** * Constructs a new instance of this class. * - * @param value The immutable value to assign to this. + * @param value the immutable value to assign to this. */ public LongValue(long value) { super(value); } + } diff --git a/container-search/src/main/java/com/yahoo/search/result/Hit.java b/container-search/src/main/java/com/yahoo/search/result/Hit.java index 6adbac56dbe..026538eab54 100644 --- a/container-search/src/main/java/com/yahoo/search/result/Hit.java +++ b/container-search/src/main/java/com/yahoo/search/result/Hit.java @@ -403,16 +403,13 @@ public class Hit extends ListenableFreezableClass implements Data, Comparable<Hi //TODO Should it be deprecated ? public final Map<String,Object> fields() { return getUnmodifiableFieldMap(); } - /** - * Will preallocate in order to avoid resizing. - * @param minSize The minimum size to reserve - */ + /** Aallocate room for the given number of fields to avoid resizing. */ public void reserve(int minSize) { getFieldMap(minSize); } /** - * Fields + * Returns an iterator over the fields of this * * @return an iterator for traversing the fields of this hit */ @@ -663,9 +660,7 @@ public class Hit extends ListenableFreezableClass implements Data, Comparable<Hi if (p == null) { return null; } else if (p instanceof HitField) { - HitField hf = (HitField) p; - - return hf.quotedContent(false); + return ((HitField)p).quotedContent(false); } else if (p instanceof StructuredData) { return p.toString(); } else if (p instanceof XMLString || p instanceof JSONString) { @@ -743,9 +738,10 @@ public class Hit extends ListenableFreezableClass implements Data, Comparable<Hi } /** - * For vespa internal use only. + * For internal use only. * Gives access to the modifiable backing set of filled summaries. * This set might be unmodifiable if the size is less than or equal to 1 + * * @return the set of filled summaries. */ protected final Set<String> getFilledInternal() { @@ -755,6 +751,7 @@ public class Hit extends ListenableFreezableClass implements Data, Comparable<Hi private Map<String,Object> getFieldMap() { return getFieldMap(16); } + private Map<String,Object> getFieldMap(int minSize) { if (fields == null) { // Compensate for loadfactor and then some, rounded up.... diff --git a/container-search/src/main/java/com/yahoo/search/result/NanNumber.java b/container-search/src/main/java/com/yahoo/search/result/NanNumber.java index 2103583dfa0..078ac04f85e 100644 --- a/container-search/src/main/java/com/yahoo/search/result/NanNumber.java +++ b/container-search/src/main/java/com/yahoo/search/result/NanNumber.java @@ -4,14 +4,14 @@ package com.yahoo.search.result; /** * A class representing unset or undefined numeric values. * - * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a> + * @author Steinar Knutsen */ @SuppressWarnings("serial") public final class NanNumber extends Number { + public static final NanNumber NaN = new NanNumber(); - private NanNumber() { - } + private NanNumber() { } @Override public double doubleValue() { diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java index 20150fc2671..3cfd8b337fc 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/FastSearcherTestCase.java @@ -4,6 +4,7 @@ package com.yahoo.prelude.fastsearch.test; import com.google.common.collect.ImmutableList; import com.yahoo.component.chain.Chain; import com.yahoo.config.subscription.ConfigGetter; +import com.yahoo.container.handler.VipStatus; import com.yahoo.container.search.Fs4Config; import com.yahoo.fs4.mplex.*; import com.yahoo.fs4.test.QueryTestCase; @@ -14,6 +15,7 @@ import com.yahoo.prelude.fastsearch.DocumentdbInfoConfig; import com.yahoo.container.protect.Error; import com.yahoo.fs4.*; import com.yahoo.prelude.fastsearch.test.fs4mock.MockBackend; +import com.yahoo.prelude.fastsearch.test.fs4mock.MockFS4ResourcePool; import com.yahoo.prelude.fastsearch.test.fs4mock.MockFSChannel; import com.yahoo.processing.execution.Execution.Trace; import com.yahoo.search.Query; @@ -111,30 +113,45 @@ public class FastSearcherTestCase { .summaryclass(new DocumentdbInfoConfig.Documentdb.Summaryclass.Builder().name("simple").id(7)) .rankprofile(new DocumentdbInfoConfig.Documentdb.Rankprofile.Builder() .name("simpler").hasRankFeatures(false).hasSummaryFeatures(false)))); + + List<SearchCluster.Node> nodes = new ArrayList<>(); + nodes.add(new SearchCluster.Node("host1", 5000, 0)); + nodes.add(new SearchCluster.Node("host2", 5000, 0)); + + MockFS4ResourcePool mockFs4ResourcePool = new MockFS4ResourcePool(); FastSearcher fastSearcher = new FastSearcher(new MockBackend(), - new FS4ResourcePool(1), - new MockDispatcher(Collections.emptyList()), + mockFs4ResourcePool, + new MockDispatcher(nodes, mockFs4ResourcePool, 1, new VipStatus()), new SummaryParameters(null), new ClusterParams("testhittype"), - new CacheParams(100, 1e64), + new CacheParams(0, 0), documentdbConfigWithOneDb); - String query = "?query=sddocname:a&dispatch.summaries"; - Result result = doSearch(fastSearcher,new Query(query), 0, 10); - ErrorMessage message = result.hits().getError(); - - assertNotNull("Got error", message); - assertEquals("Invalid query parameter", message.getMessage()); - assertEquals("When using dispatch.summaries and your summary/rankprofile require the query, you need to enable ranking.queryCache.", message.getDetailedMessage()); - assertEquals(Error.INVALID_QUERY_PARAMETER.code, message.getCode()); + { // No direct.summaries + String query = "?query=sddocname:a&summary=simple"; + Result result = doSearch(fastSearcher, new Query(query), 0, 10); + doFill(fastSearcher, result); + ErrorMessage error = result.hits().getError(); + assertNull("Since we don't route to the dispatcher we hit the mock backend, so no error", error); + } - query = "?query=sddocname:a&dispatch.summaries&ranking.queryCache"; - result = doSearch(fastSearcher,new Query(query), 0, 10); - assertNull(result.hits().getError()); + { // direct.summaries due to query cache + String query = "?query=sddocname:a&ranking.queryCache"; + Result result = doSearch(fastSearcher, new Query(query), 0, 10); + doFill(fastSearcher, result); + ErrorMessage error = result.hits().getError(); + assertEquals("Since we don't actually run summary backends we get this error when the Dispatcher is used", + "Error response from rpc node connection to host1:0: Connection error", error.getDetailedMessage()); + } - query = "?query=sddocname:a&dispatch.summaries&summary=simple&ranking=simpler"; - result = doSearch(fastSearcher,new Query(query), 0, 10); - assertNull(result.hits().getError()); + { // direct.summaries due to no summary features + String query = "?query=sddocname:a&dispatch.summaries&summary=simple&ranking=simpler"; + Result result = doSearch(fastSearcher, new Query(query), 0, 10); + doFill(fastSearcher, result); + ErrorMessage error = result.hits().getError(); + assertEquals("Since we don't actually run summary backends we get this error when the Dispatcher is used", + "Error response from rpc node connection to host1:0: Connection error", error.getDetailedMessage()); + } } @Test @@ -281,7 +298,7 @@ public class FastSearcherTestCase { assertEquals(100, fastSearcher.getCacheControl().capacity()); // Default cache =100MB - Query query = new Query("?query=ignored"); + Query query = new Query("?query=ignored&dispatch.summaries=false"); query.getRanking().setQueryCache(true); Result result = doSearch(fastSearcher, query, 0, 10); @@ -321,7 +338,6 @@ public class FastSearcherTestCase { answer.flip(); answer.get(expected); - assertEquals(expected.length, actual.length); for (int i = 0; i < expected.length; ++i) { if (expected[i] == IGNORE) { actual[i] = IGNORE; diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/fs4mock/MockFSChannel.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/fs4mock/MockFSChannel.java index 8d705406774..37a74831e56 100644 --- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/fs4mock/MockFSChannel.java +++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/test/fs4mock/MockFSChannel.java @@ -96,13 +96,13 @@ public class MockFSChannel extends FS4Channel { && lastQueryPacket.getLastOffset() >= 1) { result.addDocument( new DocumentInfo(DocsumDefinitionTestCase.createGlobalId(123), - 2003, 234, 1000)); + 2003, 234, 0)); } if (lastQueryPacket.getOffset() <= 1 && lastQueryPacket.getLastOffset() >= 2) { result.addDocument( new DocumentInfo(DocsumDefinitionTestCase.createGlobalId(456), - 1855, 234, 1001)); + 1855, 234, 1)); } packets.add(result); } diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/FillTestCase.java b/container-search/src/test/java/com/yahoo/search/dispatch/FillTestCase.java index 9938498fa1a..ed421be1947 100644 --- a/container-search/src/test/java/com/yahoo/search/dispatch/FillTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/dispatch/FillTestCase.java @@ -2,6 +2,11 @@ package com.yahoo.search.dispatch; import com.yahoo.compress.CompressionType; +import com.yahoo.log.event.Collection; +import com.yahoo.prelude.fastsearch.DocsumDefinition; +import com.yahoo.prelude.fastsearch.DocsumDefinitionSet; +import com.yahoo.prelude.fastsearch.DocsumField; +import com.yahoo.prelude.fastsearch.DocumentDatabase; import com.yahoo.prelude.fastsearch.FastHit; import com.yahoo.search.Query; import com.yahoo.search.Result; @@ -10,7 +15,10 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; @@ -44,7 +52,8 @@ public class FillTestCase { client.setDocsumReponse("host1", 2, "summaryClass1", map("field1", "s.1.2", "field2", 2)); client.setDocsumReponse("host2", 3, "summaryClass1", map("field1", "s.2.3", "field2", 3)); client.setDocsumReponse("host0", 4, "summaryClass1", map("field1", "s.0.4", "field2", 4)); - dispatcher.fill(result, "summaryClass1", CompressionType.valueOf("LZ4")); + + dispatcher.fill(result, "summaryClass1", db(), CompressionType.valueOf("LZ4")); assertEquals("s.0.0", result.hits().get("hit:0").getField("field1").toString()); assertEquals("s.2.1", result.hits().get("hit:1").getField("field1").toString()); @@ -79,7 +88,7 @@ public class FillTestCase { client.setDocsumReponse("host1", 2, "summaryClass1", new HashMap<>()); client.setDocsumReponse("host2", 3, "summaryClass1", map("field1", "s.2.3", "field2", 3)); client.setDocsumReponse("host0", 4, "summaryClass1",new HashMap<>()); - dispatcher.fill(result, "summaryClass1", CompressionType.valueOf("LZ4")); + dispatcher.fill(result, "summaryClass1", db(), CompressionType.valueOf("LZ4")); assertEquals("s.0.0", result.hits().get("hit:0").getField("field1").toString()); assertEquals("s.2.1", result.hits().get("hit:1").getField("field1").toString()); @@ -108,11 +117,20 @@ public class FillTestCase { Result result = new Result(query); result.hits().add(createHit(0, 0)); - dispatcher.fill(result, "summaryClass1", CompressionType.valueOf("LZ4")); + dispatcher.fill(result, "summaryClass1", db(), CompressionType.valueOf("LZ4")); assertEquals("Malfunctioning", result.hits().getError().getDetailedMessage()); } + private DocumentDatabase db() { + List<DocsumField> fields = new ArrayList<>(); + fields.add(DocsumField.create("field1", "string")); + fields.add(DocsumField.create("field2", "int64")); + DocsumDefinitionSet docsums = new DocsumDefinitionSet(Collections.singleton(new DocsumDefinition("summaryClass1", + fields))); + return new DocumentDatabase("default", docsums, Collections.emptySet()); + } + private FastHit createHit(int sourceNodeId, int hitId) { FastHit hit = new FastHit("hit:" + hitId, 1.0); hit.setPartId(sourceNodeId, 0); diff --git a/defaults/src/vespa/defaults.cpp b/defaults/src/vespa/defaults.cpp index d10e4064f83..05e621d2dc2 100644 --- a/defaults/src/vespa/defaults.cpp +++ b/defaults/src/vespa/defaults.cpp @@ -100,6 +100,12 @@ void findDefaults() { } env = getenv("VESPA_CONFIGSERVERS"); if (env == NULL || *env == '\0') { + env = getenv("services__addr_configserver"); + } + if (env == NULL || *env == '\0') { + env = getenv("vespa_base__addr_configserver"); + } + if (env == NULL || *env == '\0') { env = getenv("addr_configserver"); } if (env != NULL && *env != '\0') { 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 38fa91b4831..f1232259ced 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 @@ -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 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.jdisc.core; import com.google.inject.AbstractModule; @@ -17,8 +17,6 @@ import com.yahoo.jdisc.service.ServerProvider; import java.net.URI; import java.util.Map; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.logging.Logger; /** * @author Simon Thoresen @@ -26,8 +24,6 @@ import java.util.logging.Logger; */ public class ActiveContainer extends AbstractResource implements CurrentContainer { - private static final Logger log = Logger.getLogger(ActiveContainer.class.getName()); - private final ContainerTermination termination; private final Injector guiceInjector; private final Iterable<ServerProvider> serverProviders; @@ -36,7 +32,6 @@ public class ActiveContainer extends AbstractResource implements CurrentContaine private final Map<String, BindingSet<RequestHandler>> clientBindings; private final BindingSetSelector bindingSetSelector; private final TimeoutManagerImpl timeoutMgr; - private final Destructor destructor; public ActiveContainer(ContainerBuilder builder) { serverProviders = builder.serverProviders().activate(); @@ -61,16 +56,14 @@ public class ActiveContainer extends AbstractResource implements CurrentContaine }); guiceInjector = builder.guiceModules().activate(); termination = new ContainerTermination(builder.appContext()); - destructor = new Destructor(resourceReferences, timeoutMgr, termination); } @Override protected void destroy() { - boolean alreadyDestructed = destructor.destruct(); - if (alreadyDestructed) { - throw new IllegalStateException( - "Already destructed! This should not occur unless destroy have been called directly!"); - } + resourceReferences.release(); + timeoutMgr.shutdown(); + termination.run(); + super.destroy(); } /** @@ -126,40 +119,4 @@ public class ActiveContainer extends AbstractResource implements CurrentContaine return new ContainerSnapshot(this, serverBindings, clientBindings); } - // TODO Rewrite to use cleaners after Java 9 migration - @Override - protected void finalize() throws Throwable { - boolean alreadyDestructed = destructor.destruct(); - if (!alreadyDestructed) { - log.severe(toString() + " was not correctly cleaned up " + - "because of a resource leak or invalid use of reference counting."); - } - super.finalize(); - } - - // NOTE: An instance of this class must never contain a reference to the outer class (ActiveContainer). - private static class Destructor { - private final ResourcePool resourceReferences; - private final TimeoutManagerImpl timeoutMgr; - private final ContainerTermination termination; - private final AtomicBoolean done = new AtomicBoolean(); - - private Destructor(ResourcePool resourceReferences, - TimeoutManagerImpl timeoutMgr, - ContainerTermination termination) { - this.resourceReferences = resourceReferences; - this.timeoutMgr = timeoutMgr; - this.termination = termination; - } - - boolean destruct() { - boolean alreadyDestructed = this.done.getAndSet(true); - if (!alreadyDestructed) { - resourceReferences.release(); - timeoutMgr.shutdown(); - termination.run(); - } - return alreadyDestructed; - } - } } diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationEnvironmentModule.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationEnvironmentModule.java index b7cab923454..6c09cdf92f7 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationEnvironmentModule.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ApplicationEnvironmentModule.java @@ -8,6 +8,7 @@ import com.yahoo.jdisc.application.ContainerBuilder; import com.yahoo.jdisc.application.ContainerThread; import com.yahoo.jdisc.application.OsgiFramework; import com.yahoo.jdisc.service.CurrentContainer; +import com.yahoo.jdisc.statistics.ContainerWatchdogMetrics; import java.util.concurrent.ThreadFactory; @@ -28,6 +29,7 @@ class ApplicationEnvironmentModule extends AbstractModule { bind(CurrentContainer.class).toInstance(loader); bind(OsgiFramework.class).toInstance(loader.osgiFramework()); bind(ThreadFactory.class).to(ContainerThread.Factory.class); + bind(ContainerWatchdogMetrics.class).toInstance(loader.getContainerWatchdogMetrics()); } @Provides 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 81eb5815a01..3ba48ffd8cd 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 @@ -15,6 +15,7 @@ import com.yahoo.jdisc.application.OsgiFramework; import com.yahoo.jdisc.application.OsgiHeader; import com.yahoo.jdisc.service.ContainerNotReadyException; import com.yahoo.jdisc.service.CurrentContainer; +import com.yahoo.jdisc.statistics.ContainerWatchdogMetrics; import org.osgi.framework.Bundle; import org.osgi.framework.BundleContext; import org.osgi.framework.BundleException; @@ -40,6 +41,7 @@ public class ApplicationLoader implements BootstrapLoader, ContainerActivator, C private final AtomicReference<ActiveContainer> containerRef = new AtomicReference<>(); private final Object appLock = new Object(); private final List<Bundle> appBundles = new ArrayList<>(); + private final ContainerWatchdog watchdog = new ContainerWatchdog(); private Application application; private ApplicationInUseTracker applicationInUseTracker; @@ -68,6 +70,7 @@ public class ApplicationLoader implements BootstrapLoader, ContainerActivator, C next.retainReference(applicationInUseTracker); } + watchdog.onContainerActivation(next); prev = containerRef.getAndSet(next); if (prev == null) { return null; @@ -193,8 +196,9 @@ public class ApplicationLoader implements BootstrapLoader, ContainerActivator, C public void destroy() { log.finer("Destroying application loader."); try { + watchdog.close(); osgiFramework.stop(); - } catch (BundleException e) { + } catch (BundleException | InterruptedException e) { e.printStackTrace(); } } @@ -205,6 +209,10 @@ public class ApplicationLoader implements BootstrapLoader, ContainerActivator, C } } + public ContainerWatchdogMetrics getContainerWatchdogMetrics() { + return watchdog; + } + public OsgiFramework osgiFramework() { return osgiFramework; } diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/core/ContainerWatchdog.java b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ContainerWatchdog.java new file mode 100644 index 00000000000..30aa0028465 --- /dev/null +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/core/ContainerWatchdog.java @@ -0,0 +1,126 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.core; + +import com.yahoo.jdisc.Metric; +import com.yahoo.jdisc.statistics.ContainerWatchdogMetrics; + +import java.time.Clock; +import java.time.Duration; +import java.time.Instant; +import java.util.Iterator; +import java.util.LinkedList; +import java.util.List; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import java.util.logging.Logger; + +/** + * A watchdog that monitors all deactivated {@link ActiveContainer} instances with the purpose of detecting stale containers. + * + * @author bjorncs + */ +class ContainerWatchdog implements ContainerWatchdogMetrics, AutoCloseable { + + static final Duration GRACE_PERIOD = Duration.ofMinutes(30); + static final Duration UPDATE_PERIOD = Duration.ofMinutes(5); + + private static final Logger log = Logger.getLogger(ContainerWatchdog.class.getName()); + + private final Object monitor = new Object(); + private final List<DeactivatedContainer> deactivatedContainers = new LinkedList<>(); + private final ScheduledExecutorService scheduler; + private final Clock clock; + + private ActiveContainer currentContainer; + private Instant currentContainerActivationTime; + private int numStaleContainers; + + ContainerWatchdog() { + this(new ScheduledThreadPoolExecutor( + 1, + runnable -> { + Thread thread = new Thread(runnable, "container-watchdog"); + thread.setDaemon(true); + return thread; + }), + Clock.systemUTC()); + } + + ContainerWatchdog(ScheduledExecutorService scheduler, Clock clock) { + this.scheduler = scheduler; + this.clock = clock; + scheduler.scheduleAtFixedRate( + this::monitorDeactivatedContainers, UPDATE_PERIOD.getSeconds(), UPDATE_PERIOD.getSeconds(), TimeUnit.SECONDS); + } + + @Override + public void emitMetrics(Metric metric) { + int numStaleContainers; + synchronized (monitor) { + numStaleContainers = this.numStaleContainers; + } + metric.set(TOTAL_DEACTIVATED_CONTAINERS, numStaleContainers, null); + } + + @Override + public void close() throws InterruptedException { + scheduler.shutdownNow(); + scheduler.awaitTermination(1, TimeUnit.MINUTES); + synchronized (monitor) { + deactivatedContainers.clear(); + currentContainer = null; + currentContainerActivationTime = null; + } + } + + void onContainerActivation(ActiveContainer nextContainer) { + synchronized (monitor) { + if (currentContainer != null) { + deactivatedContainers.add( + new DeactivatedContainer(currentContainer, currentContainerActivationTime, clock.instant())); + } + currentContainer = nextContainer; + currentContainerActivationTime = clock.instant(); + } + } + + void monitorDeactivatedContainers() { + synchronized (monitor) { + int numStaleContainer = 0; + Iterator<DeactivatedContainer> iterator = deactivatedContainers.iterator(); + while (iterator.hasNext()) { + DeactivatedContainer container = iterator.next(); + int refCount = container.instance.retainCount(); + if (refCount == 0) { + iterator.remove(); + break; + } + if (isPastGracePeriod(container)) { + ++numStaleContainer; + log.warning( + String.format( + "Deactivated container still alive: instance=%s, activated=%s, deactivated=%s, ref-count=%d", + container.instance.toString(), container.timeActivated, container.timeDeactivated, refCount)); + } + } + this.numStaleContainers = numStaleContainer; + } + } + + private boolean isPastGracePeriod(DeactivatedContainer container) { + return clock.instant().isAfter(container.timeDeactivated.plus(GRACE_PERIOD)); + } + + private static class DeactivatedContainer { + final ActiveContainer instance; + final Instant timeActivated; + final Instant timeDeactivated; + + DeactivatedContainer(ActiveContainer instance, Instant timeActivated, Instant timeDeactivated) { + this.instance = instance; + this.timeActivated = timeActivated; + this.timeDeactivated = timeDeactivated; + } + } +} diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/statistics/ContainerWatchdogMetrics.java b/jdisc_core/src/main/java/com/yahoo/jdisc/statistics/ContainerWatchdogMetrics.java new file mode 100644 index 00000000000..2a286aef990 --- /dev/null +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/statistics/ContainerWatchdogMetrics.java @@ -0,0 +1,18 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.statistics; + +import com.yahoo.jdisc.Metric; +import com.yahoo.jdisc.core.ActiveContainer; + +/** + * Tracks statistics on stale {@link ActiveContainer} instances. + * + * @author bjorncs + */ +public interface ContainerWatchdogMetrics { + + String TOTAL_DEACTIVATED_CONTAINERS = "jdisc.deactivated_containers.total"; + + void emitMetrics(Metric metric); + +} diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/statistics/package-info.java b/jdisc_core/src/main/java/com/yahoo/jdisc/statistics/package-info.java new file mode 100644 index 00000000000..cebe03c5103 --- /dev/null +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/statistics/package-info.java @@ -0,0 +1,4 @@ +// Copyright 2018 Yahoo Holdings. 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.statistics; diff --git a/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerWatchdogTest.java b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerWatchdogTest.java new file mode 100644 index 00000000000..94e7c3a1d22 --- /dev/null +++ b/jdisc_core/src/test/java/com/yahoo/jdisc/core/ContainerWatchdogTest.java @@ -0,0 +1,73 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.jdisc.core; + +import com.yahoo.jdisc.Metric; +import com.yahoo.jdisc.test.TestDriver; +import com.yahoo.test.ManualClock; +import org.junit.Test; + +import java.time.Duration; +import java.time.Instant; +import java.util.Map; +import java.util.concurrent.ScheduledExecutorService; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; + +/** + * @author bjorncs + */ +public class ContainerWatchdogTest { + + @Test + public void watchdog_counts_stale_container() { + TestDriver driver = TestDriver.newSimpleApplicationInstanceWithoutOsgi(); + ManualClock clock = new ManualClock(Instant.EPOCH); + DummyMetric metric = new DummyMetric(); + ContainerWatchdog watchdog = new ContainerWatchdog(mock(ScheduledExecutorService.class), clock); + + ActiveContainer containerWithoutRetainedResources = new ActiveContainer(driver.newContainerBuilder()); + + watchdog.onContainerActivation(containerWithoutRetainedResources); + assertEquals(0, runMonitorStepAndGetStaleContainerCount(watchdog, metric)); + + clock.advance(Duration.ofHours(1)); + watchdog.onContainerActivation(null); + assertEquals(0, runMonitorStepAndGetStaleContainerCount(watchdog, metric)); + + clock.advance(ContainerWatchdog.GRACE_PERIOD); + assertEquals(0, runMonitorStepAndGetStaleContainerCount(watchdog, metric)); + + clock.advance(Duration.ofSeconds(1)); + assertEquals(1, runMonitorStepAndGetStaleContainerCount(watchdog, metric)); + + containerWithoutRetainedResources.release(); + assertEquals(0, runMonitorStepAndGetStaleContainerCount(watchdog, metric)); + } + + private static int runMonitorStepAndGetStaleContainerCount(ContainerWatchdog watchdog, DummyMetric metric) { + watchdog.monitorDeactivatedContainers(); + watchdog.emitMetrics(metric); + return metric.value; + } + + private static class DummyMetric implements Metric { + int value; + + @Override + public void set(String key, Number val, Context ctx) { + this.value = val.intValue(); + } + + @Override + public void add(String key, Number val, Context ctx) { + throw new UnsupportedOperationException(); + } + + @Override + public Context createContext(Map<String, ?> properties) { + throw new UnsupportedOperationException(); + } + } +} + diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/package-info.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/package-info.java new file mode 100644 index 00000000000..cfd932daf0d --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/package-info.java @@ -0,0 +1,5 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +@ExportPackage +package com.yahoo.vespa.hosted.node.admin.docker; + +import com.yahoo.osgi.annotation.ExportPackage; diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/package-info.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/package-info.java new file mode 100644 index 00000000000..e679cd259a3 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/package-info.java @@ -0,0 +1,5 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +@ExportPackage +package com.yahoo.vespa.hosted.node.admin.maintenance.acl; + +import com.yahoo.osgi.annotation.ExportPackage; diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/package-info.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/package-info.java new file mode 100644 index 00000000000..f868d657795 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/package-info.java @@ -0,0 +1,5 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +@ExportPackage +package com.yahoo.vespa.hosted.node.admin.maintenance; + +import com.yahoo.osgi.annotation.ExportPackage; diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/package-info.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/package-info.java new file mode 100644 index 00000000000..e607a270bc8 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/package-info.java @@ -0,0 +1,5 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +@ExportPackage +package com.yahoo.vespa.hosted.node.admin.nodeadmin; + +import com.yahoo.osgi.annotation.ExportPackage; diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index daf9010513f..26242961754 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java @@ -141,7 +141,12 @@ public class NodeAgentImpl implements NodeAgent { this.lastConverge = clock.instant(); this.loopThread = new Thread(() -> { - while (!terminated.get()) tick(); + try { + while (!terminated.get()) tick(); + } catch (Throwable t) { + logger.error("Unhandled throwable, taking down system.", t); + System.exit(234); + } }); this.loopThread.setName("tick-" + hostname); } @@ -416,11 +421,15 @@ public class NodeAgentImpl implements NodeAgent { isFrozenCopy = isFrozen; } + doAtTickStart(isFrozen); + boolean converged = false; + if (isFrozenCopy) { addDebugMessage("tick: isFrozen"); } else { try { converge(); + converged = true; } catch (OrchestratorException e) { logger.info(e.getMessage()); addDebugMessage(e.getMessage()); @@ -431,11 +440,10 @@ public class NodeAgentImpl implements NodeAgent { numberOfUnhandledException++; logger.error("Unhandled exception, ignoring.", e); addDebugMessage(e.getMessage()); - } catch (Throwable t) { - logger.error("Unhandled throwable, taking down system.", t); - System.exit(234); } } + + doAtTickEnd(converged); } // Public for testing @@ -492,6 +500,9 @@ public class NodeAgentImpl implements NodeAgent { } runLocalResumeScriptIfNeeded(node); + + doBeforeConverge(node); + // Because it's more important to stop a bad release from rolling out in prod, // we put the resume call last. So if we fail after updating the node repo attributes // but before resume, the app may go through the tenant pipeline but will halt in prod. @@ -526,6 +537,39 @@ public class NodeAgentImpl implements NodeAgent { } } + /** + * Execute at start of tick + * + * WARNING: MUST NOT throw an exception + * + * @param frozen whether the agent is frozen + */ + protected void doAtTickStart(boolean frozen) {} + + /** + * Execute at end of tick + * + * WARNING: MUST NOT throw an exception + * + * @param converged Whether the tick converged: converge() was called without exception + */ + protected void doAtTickEnd(boolean converged) {} + + /** + * Execute at end of a (so far) successful converge of an active node + * + * Method a subclass can override to execute code: + * - Called right before the node repo is updated with converged attributes, and + * Orchestrator resume() is called + * - The only way to avoid a successful converge and the update to the node repo + * and Orchestrator is to throw an exception + * - The method is only called in a tick if the node is active, not frozen, and + * there are no prior phases of the converge that fails + * + * @throws RuntimeException to fail the convergence + */ + protected void doBeforeConverge(NodeSpec node) {} + private void stopFilebeatSchedulerIfNeeded() { if (currentFilebeatRestarter.isPresent()) { currentFilebeatRestarter.get().cancel(true); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/package-info.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/package-info.java new file mode 100644 index 00000000000..cb2cc8589f1 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/package-info.java @@ -0,0 +1,5 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +@ExportPackage +package com.yahoo.vespa.hosted.node.admin.nodeagent; + +import com.yahoo.osgi.annotation.ExportPackage; diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/systemd/SystemCtl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/systemd/SystemCtl.java index 073978530a5..351856c4852 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/systemd/SystemCtl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/systemd/SystemCtl.java @@ -101,7 +101,7 @@ public class SystemCtl { } } - private abstract class SystemCtlCommand { + public abstract class SystemCtlCommand { private final String command; private final String unit; diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/Yum.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/Yum.java index a9d6d779f37..d88c6f4ab33 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/Yum.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/Yum.java @@ -23,11 +23,9 @@ public class Yum { private static final Pattern UNKNOWN_PACKAGE_PATTERN = Pattern.compile( "(?dm)^No package ([^ ]+) available\\.$"); - private final TaskContext taskContext; private final Terminal terminal; - public Yum(TaskContext taskContext, Terminal terminal) { - this.taskContext = taskContext; + public Yum(Terminal terminal) { this.terminal = terminal; } @@ -50,7 +48,6 @@ public class Yum { String[] packages, Pattern noopPattern) { return new GenericYumCommand( - taskContext, terminal, yumCommand, Arrays.asList(packages), @@ -58,19 +55,16 @@ public class Yum { } public static class GenericYumCommand { - private final TaskContext taskContext; private final Terminal terminal; private final String yumCommand; private final List<String> packages; private final Pattern commandOutputNoopPattern; private Optional<String> enabledRepo = Optional.empty(); - private GenericYumCommand(TaskContext taskContext, - Terminal terminal, + private GenericYumCommand(Terminal terminal, String yumCommand, List<String> packages, Pattern commandOutputNoopPattern) { - this.taskContext = taskContext; this.terminal = terminal; this.yumCommand = yumCommand; this.packages = packages; @@ -87,7 +81,7 @@ public class Yum { return this; } - public boolean converge() { + public boolean converge(TaskContext taskContext) { CommandLine commandLine = terminal.newCommandLine(taskContext); commandLine.add("yum", yumCommand, "--assumeyes"); enabledRepo.ifPresent(repo -> commandLine.add("--enablerepo=" + repo)); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTest.java index 39eabc5c512..e5bec3c912d 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/yum/YumTest.java @@ -14,8 +14,8 @@ import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; public class YumTest { - TaskContext taskContext = mock(TaskContext.class); - TestTerminal terminal = new TestTerminal(); + private TaskContext taskContext = mock(TaskContext.class); + private TestTerminal terminal = new TestTerminal(); @Before public void tearDown() { @@ -29,11 +29,11 @@ public class YumTest { 0, "foobar\nNothing to do\n"); - Yum yum = new Yum(taskContext, terminal); + Yum yum = new Yum(terminal); assertFalse(yum .install("package-1", "package-2") .enableRepo("repo-name") - .converge()); + .converge(taskContext)); } @Test @@ -43,9 +43,9 @@ public class YumTest { 0, "foobar\nNo packages marked for update\n"); - assertFalse(new Yum(taskContext, terminal) + assertFalse(new Yum(terminal) .upgrade("package-1", "package-2") - .converge()); + .converge(taskContext)); } @Test @@ -55,9 +55,9 @@ public class YumTest { 0, "foobar\nNo Packages marked for removal\n"); - assertFalse(new Yum(taskContext, terminal) + assertFalse(new Yum(terminal) .remove("package-1", "package-2") - .converge()); + .converge(taskContext)); } @Test @@ -67,10 +67,10 @@ public class YumTest { 0, "installing, installing"); - Yum yum = new Yum(taskContext, terminal); + Yum yum = new Yum(terminal); assertTrue(yum .install("package-1", "package-2") - .converge()); + .converge(taskContext)); } @Test @@ -80,11 +80,11 @@ public class YumTest { 0, "installing, installing"); - Yum yum = new Yum(taskContext, terminal); + Yum yum = new Yum(terminal); assertTrue(yum .install("package-1", "package-2") .enableRepo("repo-name") - .converge()); + .converge(taskContext)); } @Test(expected = ChildProcessFailureException.class) @@ -94,10 +94,10 @@ public class YumTest { 1, "error"); - Yum yum = new Yum(taskContext, terminal); + Yum yum = new Yum(terminal); yum.install("package-1", "package-2") .enableRepo("repo-name") - .converge(); + .converge(taskContext); fail(); } @@ -112,11 +112,11 @@ public class YumTest { "No package package-2 available.\n" + "Nothing to do\n"); - Yum yum = new Yum(taskContext, terminal); + Yum yum = new Yum(terminal); Yum.GenericYumCommand install = yum.install("package-1", "package-2", "package-3"); try { - install.converge(); + install.converge(taskContext); fail(); } catch (Exception e) { assertTrue(e.getCause() != null); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/Authorizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/Authorizer.java index 3d72c9eaca9..3601d827ac6 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/Authorizer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/Authorizer.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.provision.restapi.v2; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.SystemName; -import com.yahoo.net.HostName; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; import org.apache.http.NameValuePair; @@ -17,8 +16,8 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.function.BiPredicate; -import java.util.function.Supplier; import java.util.stream.Collectors; /** @@ -31,16 +30,12 @@ public class Authorizer implements BiPredicate<Principal, URI> { private final SystemName system; private final NodeRepository nodeRepository; - private final Supplier<String> hostnameSupplier; + private final Set<String> whitelistedHostnames; - public Authorizer(SystemName system, NodeRepository nodeRepository) { - this(system, nodeRepository, HostName::getLocalhost); - } - - Authorizer(SystemName system, NodeRepository nodeRepository, Supplier<String> hostnameSupplier) { + public Authorizer(SystemName system, NodeRepository nodeRepository, Set<String> whitelistedHostnames) { this.system = system; this.nodeRepository = nodeRepository; - this.hostnameSupplier = hostnameSupplier; + this.whitelistedHostnames = whitelistedHostnames; } /** Returns whether principal is authorized to access given URI */ @@ -62,7 +57,7 @@ public class Authorizer implements BiPredicate<Principal, URI> { } // The host itself can access all resources - if (isLocalhost(principal)) { + if (whitelistedHostnames.contains(principal.getName())) { return true; } @@ -88,11 +83,6 @@ public class Authorizer implements BiPredicate<Principal, URI> { .orElse(false); } - /** Returns whether given principal is the hostname of this node */ - private boolean isLocalhost(Principal principal) { - return principal.getName().equals(hostnameSupplier.get()); - } - /** Returns whether principal can access all given resources */ private <T> boolean canAccessAll(List<T> resources, Principal principal, BiPredicate<T, Principal> predicate) { return !resources.isEmpty() && resources.stream().allMatch(resource -> predicate.test(resource, principal)); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilter.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilter.java index fcefe73a8b9..98623a5100c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilter.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilter.java @@ -3,13 +3,16 @@ package com.yahoo.vespa.hosted.provision.restapi.v2.filter; import com.google.inject.Inject; import com.yahoo.config.provision.Zone; +import com.yahoo.config.provisioning.NodeRepositoryConfig; import com.yahoo.jdisc.handler.ResponseHandler; import com.yahoo.jdisc.http.filter.DiscFilterRequest; import com.yahoo.jdisc.http.filter.SecurityRequestFilter; +import com.yahoo.net.HostName; import com.yahoo.vespa.athenz.tls.X509CertificateUtils; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.restapi.v2.Authorizer; import com.yahoo.vespa.hosted.provision.restapi.v2.ErrorResponse; +import org.apache.commons.lang.StringUtils; import java.net.URI; import java.security.Principal; @@ -18,6 +21,8 @@ import java.util.Optional; import java.util.function.BiConsumer; import java.util.function.BiPredicate; import java.util.logging.Logger; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** * Authorization filter for all paths in config server. @@ -32,8 +37,17 @@ public class AuthorizationFilter implements SecurityRequestFilter { private final BiConsumer<ErrorResponse, ResponseHandler> rejectAction; @Inject - public AuthorizationFilter(Zone zone, NodeRepository nodeRepository) { - this(new Authorizer(zone.system(), nodeRepository), AuthorizationFilter::logAndReject); + public AuthorizationFilter(Zone zone, NodeRepository nodeRepository, NodeRepositoryConfig nodeRepositoryConfig) { + this( + new Authorizer( + zone.system(), + nodeRepository, + Stream.concat( + Stream.of(HostName.getLocalhost()), + Stream.of(nodeRepositoryConfig.hostnameWhitelist().split(",")) + ).filter(StringUtils::isNotEmpty).collect(Collectors.toSet())), + AuthorizationFilter::logAndReject + ); } AuthorizationFilter(BiPredicate<Principal, URI> authorizer, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/AuthorizerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/AuthorizerTest.java index 330262e84be..1c23dd59ad2 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/AuthorizerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/AuthorizerTest.java @@ -15,6 +15,7 @@ import org.junit.Test; import java.net.URI; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Optional; @@ -35,7 +36,7 @@ public class AuthorizerTest { public void before() { NodeFlavors flavors = new MockNodeFlavors(); nodeRepository = new MockNodeRepository(new MockCurator(), flavors); - authorizer = new Authorizer(SystemName.main, nodeRepository, () -> "cfg1"); + authorizer = new Authorizer(SystemName.main, nodeRepository, new HashSet<>(Arrays.asList("cfg1", "cfghost1"))); { // Populate with nodes used in this test. Note that only nodes requiring node repository lookup are added here Set<String> ipAddresses = new HashSet<>(Arrays.asList("127.0.0.1", "::1")); Flavor flavor = flavors.getFlavorOrThrow("default"); @@ -102,7 +103,7 @@ public class AuthorizerTest { // Trusted services can access everything in their own system assertFalse(authorized("vespa.vespa.cd.hosting", "/")); // Wrong system - assertTrue(new Authorizer(SystemName.cd, nodeRepository).test(() -> "vespa.vespa.cd.hosting", uri("/"))); + assertTrue(new Authorizer(SystemName.cd, nodeRepository, Collections.emptySet()).test(() -> "vespa.vespa.cd.hosting", uri("/"))); assertTrue(authorized("vespa.vespa.hosting", "/")); assertTrue(authorized("vespa.vespa.hosting", "/nodes/v2/node/")); assertTrue(authorized("vespa.vespa.hosting", "/nodes/v2/node/node1")); @@ -144,6 +145,7 @@ public class AuthorizerTest { public void host_authorization() { assertTrue(authorized("cfg1", "/")); assertTrue(authorized("cfg1", "/application/v2")); + assertTrue(authorized("cfghost1", "/application/v2")); } private boolean authorized(String principal, String path) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilterTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilterTest.java index c6203c76347..536c6e4c700 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilterTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/v2/filter/AuthorizationFilterTest.java @@ -6,6 +6,7 @@ import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.Zone; +import com.yahoo.config.provisioning.NodeRepositoryConfig; import com.yahoo.vespa.curator.mock.MockCurator; import com.yahoo.vespa.hosted.provision.restapi.v2.filter.FilterTester.Request; import com.yahoo.vespa.hosted.provision.testutils.MockNodeFlavors; @@ -45,8 +46,10 @@ public class AuthorizationFilterTest { private static FilterTester filterTester(SystemName system) { Zone zone = new Zone(system, Environment.prod, RegionName.defaultName()); - return new FilterTester(new AuthorizationFilter(zone, new MockNodeRepository(new MockCurator(), - new MockNodeFlavors()))); + return new FilterTester(new AuthorizationFilter( + zone, + new MockNodeRepository(new MockCurator(), new MockNodeFlavors()), + new NodeRepositoryConfig(new NodeRepositoryConfig.Builder()))); } } diff --git a/standalone-container/src/main/scala/com/yahoo/container/standalone/CloudConfigInstallVariables.scala b/standalone-container/src/main/scala/com/yahoo/container/standalone/CloudConfigInstallVariables.scala index 363a1f6249c..c68ec29fb83 100644 --- a/standalone-container/src/main/scala/com/yahoo/container/standalone/CloudConfigInstallVariables.scala +++ b/standalone-container/src/main/scala/com/yahoo/container/standalone/CloudConfigInstallVariables.scala @@ -16,7 +16,7 @@ class CloudConfigInstallVariables extends CloudConfigOptions { import CloudConfigInstallVariables._ override val rpcPort = optionalInstallVar[Integer]("port_configserver_rpc", "services") - override val allConfigServers = installVar("addr_configserver", "services") withDefault Array[ConfigServer]() + override val allConfigServers = getConfigservers override val multiTenant = optionalInstallVar[java.lang.Boolean]("multitenant") override val zookeeperBarrierTimeout = optionalInstallVar[java.lang.Long]("zookeeper_barrier_timeout") @@ -37,6 +37,12 @@ class CloudConfigInstallVariables extends CloudConfigOptions { override val hostedVespa = optionalInstallVar[java.lang.Boolean]("hosted_vespa") override val numParallelTenantLoaders = optionalInstallVar[java.lang.Integer]("num_parallel_tenant_loaders") override val loadBalancerAddress = optionalInstallVar[java.lang.String]("load_balancer_address") + + private def getConfigservers = { + val newVar = installVar("VESPA_CONFIGSERVERS", "services") withDefault Array[ConfigServer]() + val oldVar = installVar("addr_configserver", "services") withDefault Array[ConfigServer]() + if (newVar.nonEmpty) newVar else oldVar + } } object CloudConfigInstallVariables { diff --git a/vespabase/CMakeLists.txt b/vespabase/CMakeLists.txt index 2fd8ea2701f..b9a98c1295a 100644 --- a/vespabase/CMakeLists.txt +++ b/vespabase/CMakeLists.txt @@ -10,7 +10,6 @@ vespa_install_script(src/start-cbinaries.sh vespa-transactionlog-inspect bin) vespa_install_script(src/start-cbinaries.sh vespa-vds-disktool bin) vespa_install_script(src/start-cbinaries.sh vespa-distributord sbin) vespa_install_script(src/start-cbinaries.sh vespa-dispatch sbin) -vespa_install_script(src/start-cbinaries.sh vespa-filedistributor sbin) vespa_install_script(src/start-cbinaries.sh vespa-proton sbin) vespa_install_script(src/start-cbinaries.sh vespa-storaged sbin) diff --git a/vespabase/README b/vespabase/README index c74c1ce385f..da27a4144ff 100644 --- a/vespabase/README +++ b/vespabase/README @@ -1,2 +1 @@ -This CVS module will hold some vespa-base related stuff, -such as monitoring and startup scripts. +This module holds some vespa-base related stuff, such as startup scripts. diff --git a/vespajlib/src/main/java/com/yahoo/text/XML.java b/vespajlib/src/main/java/com/yahoo/text/XML.java index 30f30b2a270..f4cd355b0e1 100644 --- a/vespajlib/src/main/java/com/yahoo/text/XML.java +++ b/vespajlib/src/main/java/com/yahoo/text/XML.java @@ -28,17 +28,18 @@ import org.xml.sax.SAXParseException; * @author Steinar Knutsen */ public class XML { + /** * The point of this weird class and the jumble of abstract methods is * linking the scan for characters that must be quoted into the quoting * table, and making it actual work to make them go out of sync again. */ private static abstract class LegalCharacters { + // To quote http://www.w3.org/TR/REC-xml/ : // Char ::= #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | // [#x10000-#x10FFFF] - final boolean isLegal(final int codepoint, final boolean escapeLow, - final int stripCodePoint, final boolean isAttribute) { + final boolean isLegal(int codepoint, boolean escapeLow, int stripCodePoint, boolean isAttribute) { if (codepoint == stripCodePoint) { return removeCodePoint(); } else if (codepoint < ' ') { @@ -75,7 +76,7 @@ public class XML { } } - private boolean quotCodePoint(final boolean isAttribute) { + private boolean quotCodePoint(boolean isAttribute) { if (isAttribute) { quoteQuot(); return false; @@ -84,7 +85,7 @@ public class XML { } } - private boolean filterCodePoint(final int codepoint) { + private boolean filterCodePoint(int codepoint) { replace(codepoint); return false; } @@ -104,7 +105,7 @@ public class XML { return false; } - private boolean ctrlEscapeCodePoint(final int codepoint) { + private boolean ctrlEscapeCodePoint(int codepoint) { ctrlEscape(codepoint); return false; } @@ -349,8 +350,7 @@ public class XML { * </ul> * with character entities. * - * @param stripCodePoint - * any occurrence of this character is removed from the string + * @param stripCodePoint any occurrence of this character is removed from the string */ public static String xmlEscape(String string, boolean isAttribute, boolean escapeLowAscii, StringBuilder buffer, int stripCodePoint) { @@ -399,8 +399,7 @@ public class XML { /** * Returns the Document of an XML file reader * - * @throws RuntimeException - * if the root Document cannot be returned + * @throws RuntimeException if the root Document cannot be returned */ public static Document getDocument(Reader reader) { try { |