diff options
93 files changed, 440 insertions, 1372 deletions
diff --git a/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java index d68b36e063c..16accb368fd 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.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.config.application; +import com.google.common.collect.ImmutableSet; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; import com.yahoo.log.LogLevel; @@ -10,8 +11,16 @@ import org.w3c.dom.Element; import org.w3c.dom.NamedNodeMap; import javax.xml.transform.TransformerException; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.logging.Logger; +import java.util.stream.Collectors; /** * Handles overrides in a XML document according to the rules defined for multi environment application packages. @@ -23,8 +32,7 @@ import java.util.logging.Logger; * 3. When multiple XML elements with the same name is specified (i.e. when specifying search or docproc chains), * the id attribute of the element is used together with the element name when applying directives * - * @author lulf - * @since 5.22 + * @author Ulf Lilleengen */ class OverrideProcessor implements PreProcessor { @@ -71,15 +79,13 @@ class OverrideProcessor implements PreProcessor { } private Context getParentContext(Element parent, Context context) { - Optional<Environment> environment = context.environment; - RegionName region = context.region; - if ( ! environment.isPresent()) { - environment = getEnvironment(parent); - } - if (region.isDefault()) { - region = getRegion(parent); - } - return Context.create(environment, region); + Set<Environment> environments = context.environments; + Set<RegionName> regions = context.regions; + if (environments.isEmpty()) + environments = getEnvironments(parent); + if (regions.isEmpty()) + regions = getRegions(parent); + return Context.create(environments, regions); } /** @@ -100,13 +106,13 @@ class OverrideProcessor implements PreProcessor { */ private void checkConsistentInheritance(List<Element> children, Context context) { for (Element child : children) { - Optional<Environment> env = getEnvironment(child); - RegionName reg = getRegion(child); - if (env.isPresent() && context.environment.isPresent() && !env.equals(context.environment)) { - throw new IllegalArgumentException("Environment in child (" + env.get() + ") differs from that inherited from parent (" + context.environment + ") at " + child); + Set<Environment> environments = getEnvironments(child); + Set<RegionName> regions = getRegions(child); + if ( ! environments.isEmpty() && ! context.environments.isEmpty() && !environments.equals(context.environments)) { + throw new IllegalArgumentException("Environments in child (" + environments + ") differs from that inherited from parent (" + context.environments + ") at " + child); } - if (!reg.isDefault() && !context.region.isDefault() && !reg.equals(context.region)) { - throw new IllegalArgumentException("Region in child (" + reg + ") differs from that inherited from parent (" + context.region + ") at " + child); + if ( ! regions.isEmpty() && ! context.regions.isEmpty() && ! regions.equals(context.regions)) { + throw new IllegalArgumentException("Regions in child (" + regions + ") differs from that inherited from parent (" + context.regions + ") at " + child); } } } @@ -118,22 +124,24 @@ class OverrideProcessor implements PreProcessor { Iterator<Element> elemIt = children.iterator(); while (elemIt.hasNext()) { Element child = elemIt.next(); - if ( ! matches(getEnvironment(child), getRegion(child))) { + if ( ! matches(getEnvironments(child), getRegions(child))) { parent.removeChild(child); elemIt.remove(); } } } - private boolean matches(Optional<Environment> elementEnvironment, RegionName elementRegion) { - if (elementEnvironment.isPresent()) { // match environment - if (! environment.equals(elementEnvironment.get())) return false; + private boolean matches(Set<Environment> elementEnvironments, Set<RegionName> elementRegions) { + if ( ! elementEnvironments.isEmpty()) { // match environment + if ( ! elementEnvironments.contains(environment)) return false; } - if ( ! elementRegion.isDefault()) { // match region - if ( ! region.equals(elementRegion)) return false; - // match region but no environment in prod only to avoid a region attribute overriding capacity policies outside prod - if ( ! elementEnvironment.isPresent() && ! environment.equals(Environment.prod)) return false; + if ( ! elementRegions.isEmpty()) { // match region + // match region in prod only + if ( environment.equals(Environment.prod) && ! elementRegions.contains(region)) return false; + + // explicit region implies prod + if ( ! environment.equals(Environment.prod) && elementEnvironments.isEmpty() ) return false; } return true; @@ -174,11 +182,11 @@ class OverrideProcessor implements PreProcessor { private int getNumberOfOverrides(Element child, Context context) { int currentMatch = 0; - Optional<Environment> elementEnvironment = hasEnvironment(child) ? getEnvironment(child) : context.environment; - RegionName elementRegion = hasRegion(child) ? getRegion(child) : context.region; - if (elementEnvironment.isPresent() && elementEnvironment.get().equals(environment)) + Set<Environment> elementEnvironments = hasEnvironment(child) ? getEnvironments(child) : context.environments; + Set<RegionName> elementRegions = hasRegion(child) ? getRegions(child) : context.regions; + if ( ! elementEnvironments.isEmpty() && elementEnvironments.contains(environment)) currentMatch++; - if ( ! elementRegion.isDefault() && elementRegion.equals(region)) + if ( ! elementRegions.isEmpty() && elementRegions.contains(region)) currentMatch++; return currentMatch; } @@ -220,20 +228,16 @@ class OverrideProcessor implements PreProcessor { return element.hasAttributeNS(XmlPreProcessor.deployNamespaceUri, ATTR_ENV); } - private Optional<Environment> getEnvironment(Element element) { + private Set<Environment> getEnvironments(Element element) { String env = element.getAttributeNS(XmlPreProcessor.deployNamespaceUri, ATTR_ENV); - if (env == null || env.isEmpty()) { - return Optional.empty(); - } - return Optional.of(Environment.from(env)); + if (env == null || env.isEmpty()) return Collections.emptySet(); + return Arrays.stream(env.split(" ")).map(Environment::from).collect(Collectors.toSet()); } - private RegionName getRegion(Element element) { + private Set<RegionName> getRegions(Element element) { String reg = element.getAttributeNS(XmlPreProcessor.deployNamespaceUri, ATTR_REG); - if (reg == null || reg.isEmpty()) { - return RegionName.defaultName(); - } - return RegionName.from(reg); + if (reg == null || reg.isEmpty()) return Collections.emptySet(); + return Arrays.stream(reg.split(" ")).map(RegionName::from).collect(Collectors.toSet()); } private Map<String, List<Element>> elementsByTagNameAndId(List<Element> children) { @@ -287,21 +291,21 @@ class OverrideProcessor implements PreProcessor { */ private static final class Context { - final Optional<Environment> environment; + final ImmutableSet<Environment> environments; - final RegionName region; + final ImmutableSet<RegionName> regions; - private Context(Optional<Environment> environment, RegionName region) { - this.environment = environment; - this.region = region; + private Context(Set<Environment> environments, Set<RegionName> regions) { + this.environments = ImmutableSet.copyOf(environments); + this.regions = ImmutableSet.copyOf(regions); } static Context empty() { - return new Context(Optional.empty(), RegionName.defaultName()); + return new Context(ImmutableSet.of(), ImmutableSet.of()); } - public static Context create(Optional<Environment> environment, RegionName region) { - return new Context(environment, region); + public static Context create(Set<Environment> environments, Set<RegionName> regions) { + return new Context(environments, regions); } } diff --git a/config-application-package/src/main/java/com/yahoo/config/application/PreProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/PreProcessor.java index d331c4432bc..f207a07d3be 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/PreProcessor.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/PreProcessor.java @@ -9,9 +9,10 @@ import java.io.IOException; /** * Performs pre-processing of XML document and returns new document that has been processed. * - * @author lulf - * @since 5.21 + * @author Ulf Lilleengen */ public interface PreProcessor { - public Document process(Document input) throws IOException, TransformerException; + + Document process(Document input) throws IOException, TransformerException; + } diff --git a/config-application-package/src/main/java/com/yahoo/config/application/PropertiesProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/PropertiesProcessor.java index 6e45460a4c4..65120c3677c 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/PropertiesProcessor.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/PropertiesProcessor.java @@ -16,7 +16,6 @@ import java.util.logging.Logger; * Handles getting properties from services.xml and replacing references to properties with their real values * * @author hmusum - * @since 5.22 */ class PropertiesProcessor implements PreProcessor { private final static Logger log = Logger.getLogger(PropertiesProcessor.class.getName()); @@ -82,8 +81,8 @@ class PropertiesProcessor implements PreProcessor { } private String replaceValue(String propertyValue) { - /* Use a list with keys sorted by length (longest key first) - Needed for replacing values where you have overlapping keys */ + // Use a list with keys sorted by length (longest key first) + // Needed for replacing values where you have overlapping keys ArrayList<String> keys = new ArrayList<>(properties.keySet()); Collections.sort(keys, Collections.reverseOrder(Comparator.comparing(String::length))); diff --git a/config-application-package/src/main/java/com/yahoo/config/application/XmlPreProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/XmlPreProcessor.java index ceef0a6730e..0bb160319c0 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/XmlPreProcessor.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/XmlPreProcessor.java @@ -22,7 +22,6 @@ import java.util.List; * and create a new Document which is based on the supplied environment and region * * @author hmusum - * @since 5.22 */ public class XmlPreProcessor { @@ -51,7 +50,7 @@ public class XmlPreProcessor { public Document run() throws ParserConfigurationException, IOException, SAXException, TransformerException { DocumentBuilder docBuilder = Xml.getPreprocessDocumentBuilder(); - final Document document = docBuilder.parse(new InputSource(xmlInput)); + Document document = docBuilder.parse(new InputSource(xmlInput)); return execute(document); } diff --git a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java index 7ca9bcf48f3..1724bad765f 100644 --- a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java +++ b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java @@ -305,6 +305,7 @@ public class FilesApplicationPackage implements ApplicationPackage { /** * Verify that two sets of search definitions are disjoint (TODO: everything except error message is very generic). + * * @param fileSds Set of search definitions from file * @param bundleSds Set of search definitions from bundles */ diff --git a/config-application-package/src/test/java/com/yahoo/config/application/IncludeProcessorTest.java b/config-application-package/src/test/java/com/yahoo/config/application/IncludeProcessorTest.java index d3c2b672ee5..a456924673d 100644 --- a/config-application-package/src/test/java/com/yahoo/config/application/IncludeProcessorTest.java +++ b/config-application-package/src/test/java/com/yahoo/config/application/IncludeProcessorTest.java @@ -13,44 +13,48 @@ import java.io.*; import java.nio.file.NoSuchFileException; /** - * @author lulf - * @since 5.22 + * @author Ulf Lilleengen */ public class IncludeProcessorTest { @Test - public void testInclude() throws IOException, SAXException, XMLStreamException, ParserConfigurationException, TransformerException { + public void testInclude() throws IOException, SAXException, ParserConfigurationException, TransformerException { File app = new File("src/test/resources/multienvapp"); DocumentBuilder docBuilder = Xml.getPreprocessDocumentBuilder(); - String expected = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?><services xmlns:deploy=\"vespa\" xmlns:preprocess=\"properties\" version=\"1.0\">\n" + + String expected = + "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>" + + "<services xmlns:deploy=\"vespa\" xmlns:preprocess=\"properties\" version=\"1.0\">\n" + " <preprocess:properties>\n" + " <qrs.port>4099</qrs.port>\n" + " <qrs.port>5000</qrs.port>\n" + " </preprocess:properties>\n" + " <preprocess:properties deploy:environment='prod'>\n" + " <qrs.port deploy:region='us-west'>5001</qrs.port>" + - " <qrs.port deploy:region='us-east'>5002</qrs.port>" + + " <qrs.port deploy:region='us-east us-central'>5002</qrs.port>" + " </preprocess:properties>\n" + " <admin version=\"2.0\">\n" + " <adminserver hostalias=\"node0\"/>\n" + " </admin>\n" + - " <admin deploy:environment=\"prod\" version=\"2.0\">\n" + + " <admin deploy:environment=\"staging prod\" deploy:region=\"us-east us-central\" version=\"2.0\">\n" + " <adminserver hostalias=\"node1\"/>\n" + " </admin>\n" + " <content id=\"foo\" version=\"1.0\">\n" + " <redundancy>1</redundancy><documents>\n" + " <document mode=\"index\" type=\"music.sd\"/>\n" + - "</documents><nodes>\n" + + " </documents><nodes>\n" + " <node distribution-key=\"0\" hostalias=\"node0\"/>\n" + - "</nodes><nodes deploy:environment=\"prod\">\n" + + " </nodes>" + + " <nodes deploy:environment=\"prod\">\n" + " <node distribution-key=\"0\" hostalias=\"node0\"/>\n" + " <node distribution-key=\"1\" hostalias=\"node1\"/>\n" + - "</nodes><nodes deploy:environment=\"prod\" deploy:region=\"us-west\">\n" + + " </nodes>" + + " <nodes deploy:environment=\"prod\" deploy:region=\"us-west\">\n" + " <node distribution-key=\"0\" hostalias=\"node0\"/>\n" + " <node distribution-key=\"1\" hostalias=\"node1\"/>\n" + " <node distribution-key=\"2\" hostalias=\"node2\"/>\n" + - "</nodes></content>\n" + + " </nodes>" + + "</content>\n" + "<jdisc id=\"stateless\" version=\"1.0\">\n" + " <search deploy:environment=\"prod\">\n" + " <chain id=\"common\">\n" + @@ -68,7 +72,7 @@ public class IncludeProcessorTest { " </nodes>\n" + "</jdisc></services>"; - Document doc = (new IncludeProcessor(app)).process(docBuilder.parse(Xml.getServices(app))); + Document doc = new IncludeProcessor(app).process(docBuilder.parse(Xml.getServices(app))); // System.out.println(Xml.documentAsString(doc)); TestBase.assertDocument(expected, doc); } @@ -77,7 +81,7 @@ public class IncludeProcessorTest { public void testRequiredIncludeIsDefault() throws ParserConfigurationException, IOException, SAXException, TransformerException { File app = new File("src/test/resources/multienvapp_failrequired"); DocumentBuilder docBuilder = Xml.getPreprocessDocumentBuilder(); - (new IncludeProcessor(app)).process(docBuilder.parse(Xml.getServices(app))); + new IncludeProcessor(app).process(docBuilder.parse(Xml.getServices(app))); } } diff --git a/config-application-package/src/test/java/com/yahoo/config/application/XmlPreprocessorTest.java b/config-application-package/src/test/java/com/yahoo/config/application/XmlPreprocessorTest.java index b20437bc259..3827fe2ad42 100644 --- a/config-application-package/src/test/java/com/yahoo/config/application/XmlPreprocessorTest.java +++ b/config-application-package/src/test/java/com/yahoo/config/application/XmlPreprocessorTest.java @@ -23,8 +23,10 @@ public class XmlPreprocessorTest { private static final File services = new File(appDir, "services.xml"); @Test - public void testPreProcessing() throws IOException, SAXException, XMLStreamException, ParserConfigurationException, TransformerException { - String expectedDev = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?><services xmlns:deploy=\"vespa\" xmlns:preprocess=\"properties\" version=\"1.0\">\n" + + public void testPreProcessing() throws IOException, SAXException, ParserConfigurationException, TransformerException { + String expectedDev = + "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>" + + "<services xmlns:deploy=\"vespa\" xmlns:preprocess=\"properties\" version=\"1.0\">\n" + " <admin version=\"2.0\">\n" + " <adminserver hostalias=\"node0\"/>\n" + " </admin>\n" + @@ -46,14 +48,40 @@ public class XmlPreprocessorTest { " </nodes>\n" + " </jdisc>\n" + "</services>"; + TestBase.assertDocument(expectedDev, new XmlPreProcessor(appDir, services, Environment.dev, RegionName.from("default")).run()); - Document docDev = (new XmlPreProcessor(appDir, services, Environment.dev, RegionName.from("default")).run()); - TestBase.assertDocument(expectedDev, docDev); - + String expectedStaging = + "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>" + + "<services xmlns:deploy=\"vespa\" xmlns:preprocess=\"properties\" version=\"1.0\">\n" + + " <admin version=\"2.0\">\n" + + " <adminserver hostalias=\"node1\"/>\n" + // Difference from dev: node1 + " </admin>\n" + + " <content id=\"foo\" version=\"1.0\">\n" + + " <redundancy>1</redundancy>\n" + + " <documents>\n" + + " <document mode=\"index\" type=\"music.sd\"/>\n" + + " </documents>\n" + + " <nodes>\n" + + " <node distribution-key=\"0\" hostalias=\"node0\"/>\n" + + " </nodes>\n" + + " </content>\n" + + " <jdisc id=\"stateless\" version=\"1.0\">\n" + + " <search/>\n" + + " <component bundle=\"foobundle\" class=\"MyFoo\" id=\"foo\"/>\n" + + "" + // Difference from dev: no TestBar + " <nodes>\n" + + " <node hostalias=\"node0\" baseport=\"5000\"/>\n" + + " </nodes>\n" + + " </jdisc>\n" + + "</services>"; + // System.out.println(Xml.documentAsString(new XmlPreProcessor(appDir, services, Environment.staging, RegionName.from("default")).run())); + TestBase.assertDocument(expectedStaging, new XmlPreProcessor(appDir, services, Environment.staging, RegionName.from("default")).run()); - String expectedUsWest = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?><services xmlns:deploy=\"vespa\" xmlns:preprocess=\"properties\" version=\"1.0\">\n" + + String expectedUsWest = + "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>" + + "<services xmlns:deploy=\"vespa\" xmlns:preprocess=\"properties\" version=\"1.0\">\n" + " <admin version=\"2.0\">\n" + - " <adminserver hostalias=\"node1\"/>\n" + + " <adminserver hostalias=\"node0\"/>\n" + " </admin>\n" + " <content id=\"foo\" version=\"1.0\">\n" + " <redundancy>1</redundancy>\n" + @@ -81,12 +109,11 @@ public class XmlPreprocessorTest { " </nodes>\n" + " </jdisc>\n" + "</services>"; + TestBase.assertDocument(expectedUsWest, new XmlPreProcessor(appDir, services, Environment.prod, RegionName.from("us-west")).run()); - Document docUsWest = (new XmlPreProcessor(appDir, services, Environment.prod, RegionName.from("us-west"))).run(); - // System.out.println(Xml.documentAsString(docUsWest)); - TestBase.assertDocument(expectedUsWest, docUsWest); - - String expectedUsEast = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?><services xmlns:deploy=\"vespa\" xmlns:preprocess=\"properties\" version=\"1.0\">\n" + + String expectedUsEastAndCentral = + "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>" + + "<services xmlns:deploy=\"vespa\" xmlns:preprocess=\"properties\" version=\"1.0\">\n" + " <admin version=\"2.0\">\n" + " <adminserver hostalias=\"node1\"/>\n" + " </admin>\n" + @@ -115,14 +142,16 @@ public class XmlPreprocessorTest { " </nodes>\n" + " </jdisc>\n" + "</services>"; - - Document docUsEast = (new XmlPreProcessor(appDir, services, Environment.prod, RegionName.from("us-east"))).run(); - TestBase.assertDocument(expectedUsEast, docUsEast); + TestBase.assertDocument(expectedUsEastAndCentral, + new XmlPreProcessor(appDir, services, Environment.prod, RegionName.from("us-east")).run()); + TestBase.assertDocument(expectedUsEastAndCentral, + new XmlPreProcessor(appDir, services, Environment.prod, RegionName.from("us-central")).run()); } @Test - public void testPropertiesWithOverlappingNames() throws IOException, SAXException, XMLStreamException, ParserConfigurationException, TransformerException { - String input = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>" + + public void testPropertiesWithOverlappingNames() throws IOException, SAXException, ParserConfigurationException, TransformerException { + String input = + "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>" + "<services xmlns:deploy=\"vespa\" xmlns:preprocess=\"properties\" version=\"1.0\">" + " <preprocess:properties>" + " <sherpa.host>gamma-usnc1.dht.yahoo.com</sherpa.host>" + @@ -146,7 +175,8 @@ public class XmlPreprocessorTest { " </admin>" + "</services>"; - String expectedProd = "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>" + + String expectedProd = + "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"no\"?>" + "<services xmlns:deploy=\"vespa\" xmlns:preprocess=\"properties\" version=\"1.0\">" + " <config name='a'>" + " <a>36000</a>" + diff --git a/config-application-package/src/test/resources/multienvapp/services.xml b/config-application-package/src/test/resources/multienvapp/services.xml index 3d4a2087c57..d0f43f0b025 100644 --- a/config-application-package/src/test/resources/multienvapp/services.xml +++ b/config-application-package/src/test/resources/multienvapp/services.xml @@ -1,5 +1,6 @@ <!-- Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> <services version='1.0' xmlns:deploy="vespa" xmlns:preprocess="properties"> + <preprocess:properties> <qrs.port>4099</qrs.port> <qrs.port>5000</qrs.port> @@ -7,17 +8,23 @@ <preprocess:properties deploy:environment='prod'> <qrs.port deploy:region='us-west'>5001</qrs.port> - <qrs.port deploy:region='us-east'>5002</qrs.port> + <qrs.port deploy:region='us-east us-central'>5002</qrs.port> </preprocess:properties> + <admin version='2.0'> <adminserver hostalias='node0'/> </admin> - <admin version='2.0' deploy:environment='prod'> + + <admin version='2.0' deploy:environment='staging prod' deploy:region='us-east us-central'> <adminserver hostalias='node1'/> </admin> + <preprocess:include file='jdisc.xml'/> + <content version='1.0' id='foo'> <preprocess:include file='content/content_foo.xml'/> </content> + <preprocess:include file='doesnotexist.xml' required='false' /> + </services> 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 d729b092670..9963429bf97 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 @@ -2,6 +2,7 @@ package com.yahoo.container.logging; import com.yahoo.container.core.AccessLogConfig; +import com.yahoo.io.NativeIO; import com.yahoo.log.LogFileDb; import java.io.File; @@ -263,15 +264,28 @@ public class LogFileHandler extends StreamHandler { numberOfRecords = 0; lastRotationTime = now; nextRotationTime = 0; //figure it out later (lazy evaluation) - if (compressOnRotation && (oldFileName != null)) { - triggerCompression(oldFileName); + if ((oldFileName != null)) { + if (compressOnRotation) { + triggerCompression(oldFileName); + } else { + NativeIO nativeIO = new NativeIO(); + nativeIO.dropFileFromCache(new File(oldFileName)); + } } } private void triggerCompression(String oldFileName) { try { + String gzippedFileName = oldFileName + ".gz"; Runtime r = Runtime.getRuntime(); - Process p = r.exec(new String[] { "gzip", oldFileName }); + StringBuilder cmd = new StringBuilder("gzip"); + cmd.append(" < "). append(oldFileName).append(" > ").append(gzippedFileName); + Process p = r.exec(cmd.toString()); + NativeIO nativeIO = new NativeIO(); + File oldFile = new File(oldFileName); + nativeIO.dropFileFromCache(oldFile); // Drop from cache in case somebody else has a reference to it preventing from dying quickly. + oldFile.delete(); + nativeIO.dropFileFromCache(new File(gzippedFileName)); // 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 } catch (IOException e) { diff --git a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/Messages50TestCase.java b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/Messages50TestCase.java index 5eca8f49967..18411fd26fa 100644 --- a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/Messages50TestCase.java +++ b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/Messages50TestCase.java @@ -344,73 +344,56 @@ public class Messages50TestCase extends MessagesTestBase { @Override public void run() { - try { - FileInputStream stream = new FileInputStream(getPath("5-cpp-DocumentSummaryMessage-1.dat")); - byte[] data = new byte[stream.available()]; - assertEquals(data.length, stream.read(data)); - - Routable routable = decode(data); - assertTrue(routable instanceof DocumentSummaryMessage); - - DocumentSummaryMessage msg = (DocumentSummaryMessage)routable; - assertEquals(0, msg.getResult().getSummaryCount()); - - stream = new FileInputStream(getPath("5-cpp-DocumentSummaryMessage-2.dat")); - data = new byte[stream.available()]; - assertEquals(data.length, stream.read(data)); - - routable = decode(data); - assertTrue(routable instanceof DocumentSummaryMessage); - - msg = (DocumentSummaryMessage)routable; - assertEquals(2, msg.getResult().getSummaryCount()); - com.yahoo.vdslib.DocumentSummary.Summary s = msg.getResult().getSummary(0); - assertEquals("doc1", s.getDocId()); - byte[] b = s.getSummary(); - assertEquals(8, b.length); - byte[] c = { 's', 'u', 'm', 'm', 'a', 'r', 'y', '1' }; - for (int i = 0; i < b.length; i++) { - assertEquals(c[i], b[i]); - } - - s = msg.getResult().getSummary(1); - assertEquals("aoc17", s.getDocId()); - b = s.getSummary(); - assertEquals(9, b.length); - byte[] d = { 's', 'u', 'm', 'm', 'a', 'r', 'y', '4', '5' }; - for (int i = 0; i < b.length; i++) { - assertEquals(d[i], b[i]); - } - - stream = new FileInputStream(getPath("5-cpp-DocumentSummaryMessage-3.dat")); - data = new byte[stream.available()]; - assertEquals(data.length, stream.read(data)); + Routable routable = deserialize("DocumentSummaryMessage-1", DocumentProtocol.MESSAGE_DOCUMENTSUMMARY, Language.CPP); + assertTrue(routable instanceof DocumentSummaryMessage); + + DocumentSummaryMessage msg = (DocumentSummaryMessage) routable; + assertEquals(0, msg.getResult().getSummaryCount()); + + routable = deserialize("DocumentSummaryMessage-2", DocumentProtocol.MESSAGE_DOCUMENTSUMMARY, Language.CPP); + assertTrue(routable instanceof DocumentSummaryMessage); + + msg = (DocumentSummaryMessage) routable; + assertEquals(2, msg.getResult().getSummaryCount()); + com.yahoo.vdslib.DocumentSummary.Summary s = msg.getResult().getSummary(0); + assertEquals("doc1", s.getDocId()); + byte[] b = s.getSummary(); + assertEquals(8, b.length); + byte[] c = {'s', 'u', 'm', 'm', 'a', 'r', 'y', '1'}; + for (int i = 0; i < b.length; i++) { + assertEquals(c[i], b[i]); + } - routable = decode(data); - assertTrue(routable instanceof DocumentSummaryMessage); + s = msg.getResult().getSummary(1); + assertEquals("aoc17", s.getDocId()); + b = s.getSummary(); + assertEquals(9, b.length); + byte[] d = {'s', 'u', 'm', 'm', 'a', 'r', 'y', '4', '5'}; + for (int i = 0; i < b.length; i++) { + assertEquals(d[i], b[i]); + } + routable = deserialize("DocumentSummaryMessage-3", DocumentProtocol.MESSAGE_DOCUMENTSUMMARY, Language.CPP); + assertTrue(routable instanceof DocumentSummaryMessage); - msg = (DocumentSummaryMessage)routable; - assertEquals(2, msg.getResult().getSummaryCount()); + msg = (DocumentSummaryMessage) routable; + assertEquals(2, msg.getResult().getSummaryCount()); - s = msg.getResult().getSummary(0); - assertEquals("aoc17", s.getDocId()); - b = s.getSummary(); - assertEquals(9, b.length); - byte[] e = { 's', 'u', 'm', 'm', 'a', 'r', 'y', '4', '5' }; - for (int i = 0; i < b.length; i++) { - assertEquals(e[i], b[i]); - } + s = msg.getResult().getSummary(0); + assertEquals("aoc17", s.getDocId()); + b = s.getSummary(); + assertEquals(9, b.length); + byte[] e = {'s', 'u', 'm', 'm', 'a', 'r', 'y', '4', '5'}; + for (int i = 0; i < b.length; i++) { + assertEquals(e[i], b[i]); + } - s = msg.getResult().getSummary(1); - assertEquals("doc1", s.getDocId()); - b = s.getSummary(); - assertEquals(8, b.length); - byte[] f = { 's', 'u', 'm', 'm', 'a', 'r', 'y', '1' }; - for (int i = 0; i < b.length; i++) { - assertEquals(f[i], b[i]); - } - } catch (IOException e) { - fail(e.toString()); + s = msg.getResult().getSummary(1); + assertEquals("doc1", s.getDocId()); + b = s.getSummary(); + assertEquals(8, b.length); + byte[] f = {'s', 'u', 'm', 'm', 'a', 'r', 'y', '1'}; + for (int i = 0; i < b.length; i++) { + assertEquals(f[i], b[i]); } } } @@ -488,21 +471,13 @@ public class Messages50TestCase extends MessagesTestBase { @Override public void run() throws Exception { - FileInputStream stream = new FileInputStream(getPath("5-cpp-SearchResultMessage-1.dat")); - byte[] data = new byte[stream.available()]; - assertEquals(data.length, stream.read(data)); - - Routable routable = decode(data); + Routable routable = deserialize("SearchResultMessage-1", DocumentProtocol.MESSAGE_SEARCHRESULT, Language.CPP); assertTrue(routable instanceof SearchResultMessage); SearchResultMessage msg = (SearchResultMessage)routable; assertEquals(0, msg.getResult().getHitCount()); - stream = new FileInputStream(getPath("5-cpp-SearchResultMessage-2.dat")); - data = new byte[stream.available()]; - assertEquals(data.length, stream.read(data)); - - routable = decode(data); + routable = deserialize("SearchResultMessage-2", DocumentProtocol.MESSAGE_SEARCHRESULT, Language.CPP); assertTrue(routable instanceof SearchResultMessage); msg = (SearchResultMessage)routable; @@ -514,11 +489,7 @@ public class Messages50TestCase extends MessagesTestBase { assertEquals(109.0, h.getRank(), 1E-6); assertEquals("doc17", h.getDocId()); - stream = new FileInputStream(getPath("5-cpp-SearchResultMessage-3.dat")); - data = new byte[stream.available()]; - assertEquals(data.length, stream.read(data)); - - routable = decode(data); + routable = deserialize("SearchResultMessage-3", DocumentProtocol.MESSAGE_SEARCHRESULT, Language.CPP); assertTrue(routable instanceof SearchResultMessage); msg = (SearchResultMessage)routable; @@ -530,11 +501,7 @@ public class Messages50TestCase extends MessagesTestBase { assertEquals(89.0, h.getRank(), 1E-6); assertEquals("doc1", h.getDocId()); - stream = new FileInputStream(getPath("5-cpp-SearchResultMessage-4.dat")); - data = new byte[stream.available()]; - assertEquals(data.length, stream.read(data)); - - routable = decode(data); + routable = deserialize("SearchResultMessage-4", DocumentProtocol.MESSAGE_SEARCHRESULT, Language.CPP); assertTrue(routable instanceof SearchResultMessage); msg = (SearchResultMessage)routable; @@ -732,21 +699,13 @@ public class Messages50TestCase extends MessagesTestBase { @Override public void run() throws Exception { - FileInputStream stream = new FileInputStream(getPath("5-cpp-QueryResultMessage-1.dat")); - byte[] data = new byte[stream.available()]; - assertEquals(data.length, stream.read(data)); - - Routable routable = decode(data); + Routable routable = deserialize("QueryResultMessage-1", DocumentProtocol.MESSAGE_QUERYRESULT, Language.CPP); assertTrue(routable instanceof QueryResultMessage); QueryResultMessage msg = (QueryResultMessage)routable; assertEquals(0, msg.getResult().getHitCount()); - stream = new FileInputStream(getPath("5-cpp-QueryResultMessage-2.dat")); - data = new byte[stream.available()]; - assertEquals(data.length, stream.read(data)); - - routable = decode(data); + routable = deserialize("QueryResultMessage-2", DocumentProtocol.MESSAGE_QUERYRESULT, Language.CPP); assertTrue(routable instanceof QueryResultMessage); msg = (QueryResultMessage)routable; @@ -758,11 +717,7 @@ public class Messages50TestCase extends MessagesTestBase { assertEquals(109.0, h.getRank(), 1E-6); assertEquals("doc17", h.getDocId()); - stream = new FileInputStream(getPath("5-cpp-QueryResultMessage-3.dat")); - data = new byte[stream.available()]; - assertEquals(data.length, stream.read(data)); - - routable = decode(data); + routable = deserialize("QueryResultMessage-3", DocumentProtocol.MESSAGE_QUERYRESULT, Language.CPP); assertTrue(routable instanceof QueryResultMessage); msg = (QueryResultMessage)routable; @@ -774,11 +729,7 @@ public class Messages50TestCase extends MessagesTestBase { assertEquals(89.0, h.getRank(), 1E-6); assertEquals("doc1", h.getDocId()); - stream = new FileInputStream(getPath("5-cpp-QueryResultMessage-4.dat")); - data = new byte[stream.available()]; - assertEquals(data.length, stream.read(data)); - - routable = decode(data); + routable = deserialize("QueryResultMessage-4", DocumentProtocol.MESSAGE_QUERYRESULT, Language.CPP); assertTrue(routable instanceof QueryResultMessage); msg = (QueryResultMessage)routable; diff --git a/documentapi/src/tests/messagebus/messagebus_test.cpp b/documentapi/src/tests/messagebus/messagebus_test.cpp index dff3f3f1bac..ea328007027 100644 --- a/documentapi/src/tests/messagebus/messagebus_test.cpp +++ b/documentapi/src/tests/messagebus/messagebus_test.cpp @@ -4,6 +4,7 @@ #include <vespa/document/datatype/documenttype.h> #include <vespa/document/fieldvalue/document.h> #include <vespa/document/repo/documenttyperepo.h> +#include <vespa/document/update/documentupdate.h> #include <vespa/documentapi/documentapi.h> #include <vespa/documentapi/loadtypes/loadtypeset.h> #include <vespa/vdslib/state/clusterstate.h> diff --git a/documentapi/src/tests/messages/messages50test.cpp b/documentapi/src/tests/messages/messages50test.cpp index 6705277d5c3..cee0af5ad8e 100644 --- a/documentapi/src/tests/messages/messages50test.cpp +++ b/documentapi/src/tests/messages/messages50test.cpp @@ -5,6 +5,7 @@ #include <vespa/document/datatype/documenttype.h> #include <vespa/document/select/parser.h> #include <vespa/document/repo/documenttyperepo.h> +#include <vespa/document/update/documentupdate.h> #include <vespa/document/update/fieldpathupdates.h> #include <vespa/documentapi/documentapi.h> #include <vespa/document/bucket/fixed_bucket_spaces.h> @@ -22,7 +23,6 @@ Messages50Test::Messages50Test() { // This list MUST mirror the list of routable factories from the DocumentProtocol constructor that support // version 5.0. When adding tests to this list, please KEEP THEM ORDERED alphabetically like they are now. - putTest(DocumentProtocol::MESSAGE_BATCHDOCUMENTUPDATE, TEST_METHOD(Messages50Test::testBatchDocumentUpdateMessage)); putTest(DocumentProtocol::MESSAGE_CREATEVISITOR, TEST_METHOD(Messages50Test::testCreateVisitorMessage)); putTest(DocumentProtocol::MESSAGE_DESTROYVISITOR, TEST_METHOD(Messages50Test::testDestroyVisitorMessage)); putTest(DocumentProtocol::MESSAGE_DOCUMENTLIST, TEST_METHOD(Messages50Test::testDocumentListMessage)); @@ -41,7 +41,6 @@ Messages50Test::Messages50Test() putTest(DocumentProtocol::MESSAGE_UPDATEDOCUMENT, TEST_METHOD(Messages50Test::testUpdateDocumentMessage)); putTest(DocumentProtocol::MESSAGE_VISITORINFO, TEST_METHOD(Messages50Test::testVisitorInfoMessage)); - putTest(DocumentProtocol::REPLY_BATCHDOCUMENTUPDATE, TEST_METHOD(Messages50Test::testBatchDocumentUpdateReply)); putTest(DocumentProtocol::REPLY_CREATEVISITOR, TEST_METHOD(Messages50Test::testCreateVisitorReply)); putTest(DocumentProtocol::REPLY_DESTROYVISITOR, TEST_METHOD(Messages50Test::testDestroyVisitorReply)); putTest(DocumentProtocol::REPLY_DOCUMENTLIST, TEST_METHOD(Messages50Test::testDocumentListReply)); @@ -259,38 +258,27 @@ Messages50Test::testDocumentSummaryMessage() EXPECT_EQUAL(srm.hasSequenceId(), false); EXPECT_EQUAL(srm.getSummaryCount(), size_t(0)); - mbus::Blob data = encode(srm); + EXPECT_EQUAL(MESSAGE_BASE_LENGTH + size_t(12), serialize("DocumentSummaryMessage-1", srm)); - EXPECT_EQUAL(MESSAGE_BASE_LENGTH + size_t(12), data.size()); - - writeFile(getPath("5-cpp-DocumentSummaryMessage-1.dat"), data); - // print(data); - - mbus::Routable::UP routable = decode(data); + mbus::Routable::UP routable = deserialize("DocumentSummaryMessage-1", DocumentProtocol::MESSAGE_DOCUMENTSUMMARY, LANG_CPP); if (!EXPECT_TRUE(routable.get() != NULL)) { return false; } - EXPECT_EQUAL(routable->getType(), (uint32_t)DocumentProtocol::MESSAGE_DOCUMENTSUMMARY); DocumentSummaryMessage * dm = static_cast<DocumentSummaryMessage *>(routable.get()); EXPECT_EQUAL(dm->getSummaryCount(), size_t(0)); srm.addSummary("doc1", "summary1", 8); srm.addSummary("aoc17", "summary45", 9); - data = encode(srm); - //print(data); - const void *summary(NULL); const char *docId(NULL); size_t sz(0); - EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 52u, data.size()); - writeFile(getPath("5-cpp-DocumentSummaryMessage-2.dat"), data); - routable = decode(data); + EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 52u, serialize("DocumentSummaryMessage-2", srm)); + routable = deserialize("DocumentSummaryMessage-2", DocumentProtocol::MESSAGE_DOCUMENTSUMMARY, LANG_CPP); if (!EXPECT_TRUE(routable.get() != NULL)) { return false; } - EXPECT_EQUAL(routable->getType(), (uint32_t)DocumentProtocol::MESSAGE_DOCUMENTSUMMARY); dm = static_cast<DocumentSummaryMessage *>(routable.get()); EXPECT_EQUAL(dm->getSummaryCount(), size_t(2)); dm->getSummary(0, docId, summary, sz); @@ -304,14 +292,11 @@ Messages50Test::testDocumentSummaryMessage() srm.sort(); - data = encode(srm); - EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 52u, data.size()); - writeFile(getPath("5-cpp-DocumentSummaryMessage-3.dat"), data); - routable = decode(data); + EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 52u, serialize("DocumentSummaryMessage-3", srm)); + routable = deserialize("DocumentSummaryMessage-3", DocumentProtocol::MESSAGE_DOCUMENTSUMMARY, LANG_CPP); if (!EXPECT_TRUE(routable.get() != NULL)) { return false; } - EXPECT_EQUAL(routable->getType(), (uint32_t)DocumentProtocol::MESSAGE_DOCUMENTSUMMARY); dm = static_cast<DocumentSummaryMessage *>(routable.get()); EXPECT_EQUAL(dm->getSummaryCount(), size_t(2)); dm->getSummary(0, docId, summary, sz); @@ -524,38 +509,25 @@ Messages50Test::testSearchResultMessage() EXPECT_EQUAL(srm.vdslib::SearchResult::getSerializedSize(), 20u); EXPECT_EQUAL(srm.getSerializedSize(), 20u); - mbus::Blob data = encode(srm); + EXPECT_EQUAL(MESSAGE_BASE_LENGTH + size_t(24), serialize("SearchResultMessage-1", srm)); - EXPECT_EQUAL(MESSAGE_BASE_LENGTH + size_t(24), data.size()); - - writeFile(getPath("5-cpp-SearchResultMessage-1.dat"), data); - // print(data); - - mbus::Routable::UP routable = decode(data); + mbus::Routable::UP routable = deserialize("SearchResultMessage-1", DocumentProtocol::MESSAGE_SEARCHRESULT, LANG_CPP); if (!EXPECT_TRUE(routable.get() != NULL)) { return false; } - EXPECT_EQUAL(routable->getType(), (uint32_t)DocumentProtocol::MESSAGE_SEARCHRESULT); SearchResultMessage * dm = static_cast<SearchResultMessage *>(routable.get()); EXPECT_EQUAL(dm->getSequenceId(), size_t(0)); EXPECT_EQUAL(dm->getHitCount(), size_t(0)); srm.addHit(0, "doc1", 89); srm.addHit(1, "doc17", 109); - //srm.setSequenceId(567); - - data = encode(srm); - //EXPECT_EQUAL(srm.getSequenceId(), size_t(567)); - EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 55u, data.size()); - writeFile(getPath("5-cpp-SearchResultMessage-2.dat"), data); - routable = decode(data); + EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 55u, serialize("SearchResultMessage-2", srm)); + routable = deserialize("SearchResultMessage-2", DocumentProtocol::MESSAGE_SEARCHRESULT, LANG_CPP); if (!EXPECT_TRUE(routable.get() != NULL)) { return false; } - EXPECT_EQUAL(routable->getType(), (uint32_t)DocumentProtocol::MESSAGE_SEARCHRESULT); dm = static_cast<SearchResultMessage *>(routable.get()); -// EXPECT_EQUAL(dm->getSequenceId(), size_t(567)); EXPECT_EQUAL(dm->getHitCount(), size_t(2)); const char *docId; SearchResultMessage::RankType rank; @@ -568,16 +540,12 @@ Messages50Test::testSearchResultMessage() srm.sort(); - data = encode(srm); - EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 55u, data.size()); - writeFile(getPath("5-cpp-SearchResultMessage-3.dat"), data); - routable = decode(data); + EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 55u, serialize("SearchResultMessage-3", srm)); + routable = deserialize("SearchResultMessage-3", DocumentProtocol::MESSAGE_SEARCHRESULT, LANG_CPP); if (!EXPECT_TRUE(routable.get() != NULL)) { return false; } - EXPECT_EQUAL(routable->getType(), (uint32_t)DocumentProtocol::MESSAGE_SEARCHRESULT); dm = static_cast<SearchResultMessage *>(routable.get()); -// EXPECT_EQUAL(dm->getSequenceId(), size_t(567)); EXPECT_EQUAL(dm->getHitCount(), size_t(2)); dm->getHit(0, docId, rank); EXPECT_EQUAL(rank, SearchResultMessage::RankType(109)); @@ -590,18 +558,13 @@ Messages50Test::testSearchResultMessage() srm2.addHit(0, "doc1", 89, "sortdata2", 9); srm2.addHit(1, "doc17", 109, "sortdata1", 9); srm2.addHit(2, "doc18", 90, "sortdata3", 9); - //srm2.setSequenceId(567); - data = encode(srm2); - EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 108u, data.size()); - writeFile(getPath("5-cpp-SearchResultMessage-4.dat"), data); - routable = decode(data); + EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 108u, serialize("SearchResultMessage-4", srm2)); + routable = deserialize("SearchResultMessage-4", DocumentProtocol::MESSAGE_SEARCHRESULT, LANG_CPP); if (!EXPECT_TRUE(routable.get() != NULL)) { return false; } - EXPECT_EQUAL(routable->getType(), (uint32_t)DocumentProtocol::MESSAGE_SEARCHRESULT); dm = static_cast<SearchResultMessage *>(routable.get()); - //EXPECT_EQUAL(dm->getSequenceId(), size_t(567)); EXPECT_EQUAL(dm->getHitCount(), size_t(3)); dm->getHit(0, docId, rank); EXPECT_EQUAL(rank, SearchResultMessage::RankType(89)); @@ -635,16 +598,12 @@ Messages50Test::testSearchResultMessage() EXPECT_EQUAL(rank, SearchResultMessage::RankType(90)); EXPECT_EQUAL(strcmp("doc18", docId), 0); - data = encode(srm2); - EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 108u, data.size()); - writeFile(getPath("5-cpp-SearchResultMessage-5.dat"), data); - routable = decode(data); + EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 108u, serialize("SearchResultMessage-5", srm2)); + routable = deserialize("SearchResultMessage-5", DocumentProtocol::MESSAGE_SEARCHRESULT, LANG_CPP); if (!EXPECT_TRUE(routable.get() != NULL)) { return false; } - EXPECT_EQUAL(routable->getType(), (uint32_t)DocumentProtocol::MESSAGE_SEARCHRESULT); dm = static_cast<SearchResultMessage *>(routable.get()); -// EXPECT_EQUAL(dm->getSequenceId(), size_t(567)); EXPECT_EQUAL(dm->getHitCount(), size_t(3)); dm->getHit(0, docId, rank); dm->getSortBlob(0, buf, sz); @@ -693,90 +652,6 @@ Messages50Test::testUpdateDocumentMessage() } bool -Messages50Test::testBatchDocumentUpdateMessage() -{ - const DocumentTypeRepo &repo = getTypeRepo(); - const document::DocumentType &docType = *repo.getDocumentType("testdoc"); - - BatchDocumentUpdateMessage msg(1234); - - { - document::DocumentUpdate::SP upd; - upd.reset(new document::DocumentUpdate(repo, docType, document::DocumentId("userdoc:footype:1234:foo"))); - upd->addFieldPathUpdate(document::FieldPathUpdate::CP( - new document::RemoveFieldPathUpdate("intfield", "testdoc.intfield > 0"))); - msg.addUpdate(upd); - } - { - document::DocumentUpdate::SP upd; - upd.reset(new document::DocumentUpdate(repo, docType, document::DocumentId("orderdoc(32,17):footype:1234:123456789:foo"))); - upd->addFieldPathUpdate(document::FieldPathUpdate::CP( - new document::RemoveFieldPathUpdate("intfield", "testdoc.intfield > 0"))); - msg.addUpdate(upd); - } - try { - document::DocumentUpdate::SP upd; - upd.reset(new document::DocumentUpdate(repo, docType, document::DocumentId("userdoc:footype:5678:foo"))); - upd->addFieldPathUpdate(document::FieldPathUpdate::CP( - new document::RemoveFieldPathUpdate("intfield", "testdoc.intfield > 0"))); - msg.addUpdate(upd); - EXPECT_TRUE(false); - } catch (...) { - } - try { - document::DocumentUpdate::SP upd; - upd.reset(new document::DocumentUpdate(repo, docType, document::DocumentId("groupdoc:footype:hable:foo"))); - upd->addFieldPathUpdate(document::FieldPathUpdate::CP( - new document::RemoveFieldPathUpdate("intfield", "testdoc.intfield > 0"))); - msg.addUpdate(upd); - EXPECT_TRUE(false); - } catch (...) { - } - - EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 202u, serialize("BatchDocumentUpdateMessage", msg)); - for (uint32_t lang = 0; lang < NUM_LANGUAGES; ++lang) { - mbus::Routable::UP obj = deserialize("BatchDocumentUpdateMessage", DocumentProtocol::MESSAGE_BATCHDOCUMENTUPDATE, lang); - if (EXPECT_TRUE(obj.get() != NULL)) { - BatchDocumentUpdateMessage &ref = static_cast<BatchDocumentUpdateMessage&>(*obj); - EXPECT_EQUAL(2u, ref.getUpdates().size()); - } - } - - return true; -} - -bool -Messages50Test::testBatchDocumentUpdateReply() -{ - BatchDocumentUpdateReply reply; - reply.setHighestModificationTimestamp(30); - { - std::vector<bool> notFound(3); - notFound[0] = false; - notFound[1] = true; - notFound[2] = true; - reply.getDocumentsNotFound() = notFound; - } - - EXPECT_EQUAL(20u, serialize("BatchDocumentUpdateReply", reply)); - - for (uint32_t lang = 0; lang < NUM_LANGUAGES; ++lang) { - mbus::Routable::UP obj = deserialize("BatchDocumentUpdateReply", DocumentProtocol::REPLY_BATCHDOCUMENTUPDATE, lang); - if (EXPECT_TRUE(obj.get() != NULL)) { - BatchDocumentUpdateReply &ref = dynamic_cast<BatchDocumentUpdateReply&>(*obj); - EXPECT_EQUAL(30u, ref.getHighestModificationTimestamp()); - { - const std::vector<bool>& notFound = ref.getDocumentsNotFound(); - EXPECT_TRUE(notFound[0] == false); - EXPECT_TRUE(notFound[1] == true); - EXPECT_TRUE(notFound[2] == true); - } - } - } - return true; -} - -bool Messages50Test::testQueryResultMessage() { QueryResultMessage srm; @@ -787,18 +662,12 @@ Messages50Test::testQueryResultMessage() EXPECT_EQUAL(sr.getSerializedSize(), 20u); EXPECT_EQUAL(srm.getApproxSize(), 28u); - mbus::Blob data = encode(srm); - - EXPECT_EQUAL(MESSAGE_BASE_LENGTH + size_t(32), data.size()); + EXPECT_EQUAL(MESSAGE_BASE_LENGTH + size_t(32), serialize("QueryResultMessage-1", srm)); - writeFile(getPath("5-cpp-QueryResultMessage-1.dat"), data); - // print(data); - - mbus::Routable::UP routable = decode(data); + mbus::Routable::UP routable = deserialize("QueryResultMessage-1", DocumentProtocol::MESSAGE_QUERYRESULT, LANG_CPP); if (!EXPECT_TRUE(routable.get() != NULL)) { return false; } - EXPECT_EQUAL(routable->getType(), (uint32_t)DocumentProtocol::MESSAGE_QUERYRESULT); QueryResultMessage * dm = static_cast<QueryResultMessage *>(routable.get()); vdslib::SearchResult * dr(&dm->getSearchResult()); EXPECT_EQUAL(dm->getSequenceId(), size_t(0)); @@ -807,15 +676,11 @@ Messages50Test::testQueryResultMessage() sr.addHit(0, "doc1", 89); sr.addHit(1, "doc17", 109); - data = encode(srm); - - EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 63u, data.size()); - writeFile(getPath("5-cpp-QueryResultMessage-2.dat"), data); - routable = decode(data); + EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 63u, serialize("QueryResultMessage-2", srm)); + routable = deserialize("QueryResultMessage-2", DocumentProtocol::MESSAGE_QUERYRESULT, LANG_CPP); if (!EXPECT_TRUE(routable.get() != NULL)) { return false; } - EXPECT_EQUAL(routable->getType(), (uint32_t)DocumentProtocol::MESSAGE_QUERYRESULT); dm = static_cast<QueryResultMessage *>(routable.get()); dr = &dm->getSearchResult(); EXPECT_EQUAL(dr->getHitCount(), size_t(2)); @@ -830,14 +695,11 @@ Messages50Test::testQueryResultMessage() sr.sort(); - data = encode(srm); - EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 63u, data.size()); - writeFile(getPath("5-cpp-QueryResultMessage-3.dat"), data); - routable = decode(data); + EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 63u, serialize("QueryResultMessage-3", srm)); + routable = deserialize("QueryResultMessage-3", DocumentProtocol::MESSAGE_QUERYRESULT, LANG_CPP); if (!EXPECT_TRUE(routable.get() != NULL)) { return false; } - EXPECT_EQUAL(routable->getType(), (uint32_t)DocumentProtocol::MESSAGE_QUERYRESULT); dm = static_cast<QueryResultMessage *>(routable.get()); dr = &dm->getSearchResult(); EXPECT_EQUAL(dr->getHitCount(), size_t(2)); @@ -853,15 +715,12 @@ Messages50Test::testQueryResultMessage() sr2.addHit(0, "doc1", 89, "sortdata2", 9); sr2.addHit(1, "doc17", 109, "sortdata1", 9); sr2.addHit(2, "doc18", 90, "sortdata3", 9); - data = encode(srm2); - EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 116u, data.size()); - writeFile(getPath("5-cpp-QueryResultMessage-4.dat"), data); - routable = decode(data); + EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 116u, serialize("QueryResultMessage-4", srm2)); + routable = deserialize("QueryResultMessage-4", DocumentProtocol::MESSAGE_QUERYRESULT, LANG_CPP); if (!EXPECT_TRUE(routable.get() != NULL)) { return false; } - EXPECT_EQUAL(routable->getType(), (uint32_t)DocumentProtocol::MESSAGE_QUERYRESULT); dm = static_cast<QueryResultMessage *>(routable.get()); dr = &dm->getSearchResult(); EXPECT_EQUAL(dr->getHitCount(), size_t(3)); @@ -897,14 +756,11 @@ Messages50Test::testQueryResultMessage() EXPECT_EQUAL(rank, vdslib::SearchResult::RankType(90)); EXPECT_EQUAL(strcmp("doc18", docId), 0); - data = encode(srm2); - EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 116u, data.size()); - writeFile(getPath("5-cpp-QueryResultMessage-5.dat"), data); - routable = decode(data); + EXPECT_EQUAL(MESSAGE_BASE_LENGTH + 116u, serialize("QueryResultMessage-5", srm2)); + routable = deserialize("QueryResultMessage-5", DocumentProtocol::MESSAGE_QUERYRESULT, LANG_CPP); if (!EXPECT_TRUE(routable.get() != NULL)) { return false; } - EXPECT_EQUAL(routable->getType(), (uint32_t)DocumentProtocol::MESSAGE_QUERYRESULT); dm = static_cast<QueryResultMessage *>(routable.get()); dr = &dm->getSearchResult(); EXPECT_EQUAL(dr->getHitCount(), size_t(3)); diff --git a/documentapi/src/tests/messages/messages50test.h b/documentapi/src/tests/messages/messages50test.h index c764f814a45..96c1be78c27 100644 --- a/documentapi/src/tests/messages/messages50test.h +++ b/documentapi/src/tests/messages/messages50test.h @@ -13,8 +13,6 @@ protected: public: Messages50Test(); - bool testBatchDocumentUpdateMessage(); - bool testBatchDocumentUpdateReply(); bool testCreateVisitorMessage(); bool testCreateVisitorReply(); bool testDestroyVisitorMessage(); diff --git a/documentapi/src/tests/messages/messages52test.cpp b/documentapi/src/tests/messages/messages52test.cpp index 8f5d7381500..1da48d4aa41 100644 --- a/documentapi/src/tests/messages/messages52test.cpp +++ b/documentapi/src/tests/messages/messages52test.cpp @@ -5,6 +5,7 @@ #include "messages52test.h" #include <vespa/documentapi/documentapi.h> #include <vespa/document/repo/documenttyperepo.h> +#include <vespa/document/update/documentupdate.h> #include <vespa/document/update/fieldpathupdates.h> #include <vespa/document/datatype/documenttype.h> diff --git a/documentapi/src/tests/policies/policies_test.cpp b/documentapi/src/tests/policies/policies_test.cpp index 3e804c30415..c01cdfde30c 100644 --- a/documentapi/src/tests/policies/policies_test.cpp +++ b/documentapi/src/tests/policies/policies_test.cpp @@ -22,6 +22,7 @@ #include <vespa/document/fieldvalue/longfieldvalue.h> #include <vespa/document/datatype/documenttype.h> #include <vespa/document/repo/documenttyperepo.h> +#include <vespa/document/update/documentupdate.h> #include <vespa/vespalib/testkit/testapp.h> #include <vespa/vespalib/util/stringfmt.h> diff --git a/documentapi/src/vespa/documentapi/documentapi.h b/documentapi/src/vespa/documentapi/documentapi.h index dff4125e624..0056dd2abdf 100644 --- a/documentapi/src/vespa/documentapi/documentapi.h +++ b/documentapi/src/vespa/documentapi/documentapi.h @@ -1,8 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once -#include <vespa/documentapi/messagebus/messages/batchdocumentupdatemessage.h> -#include <vespa/documentapi/messagebus/messages/batchdocumentupdatereply.h> #include <vespa/documentapi/messagebus/messages/getbucketstatemessage.h> #include <vespa/documentapi/messagebus/messages/getbucketstatereply.h> #include <vespa/documentapi/messagebus/messages/getdocumentmessage.h> diff --git a/documentapi/src/vespa/documentapi/messagebus/documentprotocol.cpp b/documentapi/src/vespa/documentapi/messagebus/documentprotocol.cpp index d2661d0fe6c..1a1d1d96ab6 100644 --- a/documentapi/src/vespa/documentapi/messagebus/documentprotocol.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/documentprotocol.cpp @@ -57,7 +57,6 @@ DocumentProtocol::DocumentProtocol(const LoadTypeSet& loadTypes, std::vector<vespalib::VersionSpecification> from6 = { version6 }; // Add 5.0 serialization - putRoutableFactory(MESSAGE_BATCHDOCUMENTUPDATE, IRoutableFactory::SP(new RoutableFactories50::BatchDocumentUpdateMessageFactory(*_repo)), from50); putRoutableFactory(MESSAGE_CREATEVISITOR, IRoutableFactory::SP(new RoutableFactories50::CreateVisitorMessageFactory(*_repo)), from50); putRoutableFactory(MESSAGE_DESTROYVISITOR, IRoutableFactory::SP(new RoutableFactories50::DestroyVisitorMessageFactory()), from50); putRoutableFactory(MESSAGE_DOCUMENTLIST, IRoutableFactory::SP(new RoutableFactories50::DocumentListMessageFactory(*_repo)), from50); @@ -75,7 +74,6 @@ DocumentProtocol::DocumentProtocol(const LoadTypeSet& loadTypes, putRoutableFactory(MESSAGE_STATBUCKET, IRoutableFactory::SP(new RoutableFactories50::StatBucketMessageFactory()), from50); putRoutableFactory(MESSAGE_UPDATEDOCUMENT, IRoutableFactory::SP(new RoutableFactories50::UpdateDocumentMessageFactory(*_repo)), from50); putRoutableFactory(MESSAGE_VISITORINFO, IRoutableFactory::SP(new RoutableFactories50::VisitorInfoMessageFactory()), from50); - putRoutableFactory(REPLY_BATCHDOCUMENTUPDATE, IRoutableFactory::SP(new RoutableFactories50::BatchDocumentUpdateReplyFactory()), from50); putRoutableFactory(REPLY_CREATEVISITOR, IRoutableFactory::SP(new RoutableFactories50::CreateVisitorReplyFactory()), from50); putRoutableFactory(REPLY_DESTROYVISITOR, IRoutableFactory::SP(new RoutableFactories50::DestroyVisitorReplyFactory()), from50); putRoutableFactory(REPLY_DOCUMENTLIST, IRoutableFactory::SP(new RoutableFactories50::DocumentListReplyFactory()), from50); diff --git a/documentapi/src/vespa/documentapi/messagebus/documentprotocol.h b/documentapi/src/vespa/documentapi/messagebus/documentprotocol.h index 32e6dc1d95e..79f7d7c0ccc 100644 --- a/documentapi/src/vespa/documentapi/messagebus/documentprotocol.h +++ b/documentapi/src/vespa/documentapi/messagebus/documentprotocol.h @@ -68,7 +68,6 @@ public: MESSAGE_EMPTYBUCKETS = DOCUMENT_MESSAGE + 23, MESSAGE_REMOVELOCATION = DOCUMENT_MESSAGE + 24, MESSAGE_QUERYRESULT = DOCUMENT_MESSAGE + 25, - MESSAGE_BATCHDOCUMENTUPDATE = DOCUMENT_MESSAGE + 26, // MESSAGE_GARBAGECOLLECT = DOCUMENT_MESSAGE + 27, DOCUMENT_REPLY = 200000, @@ -92,7 +91,6 @@ public: REPLY_EMPTYBUCKETS = DOCUMENT_REPLY + 23, REPLY_REMOVELOCATION = DOCUMENT_REPLY + 24, REPLY_QUERYRESULT = DOCUMENT_REPLY + 25, - REPLY_BATCHDOCUMENTUPDATE = DOCUMENT_REPLY + 26, // REPLY_GARBAGECOLLECT = DOCUMENT_REPLY + 27, REPLY_WRONGDISTRIBUTION = DOCUMENT_REPLY + 1000, REPLY_DOCUMENTIGNORED = DOCUMENT_REPLY + 1001 diff --git a/documentapi/src/vespa/documentapi/messagebus/messages/CMakeLists.txt b/documentapi/src/vespa/documentapi/messagebus/messages/CMakeLists.txt index 3fa7e6086e9..7bd18a14719 100644 --- a/documentapi/src/vespa/documentapi/messagebus/messages/CMakeLists.txt +++ b/documentapi/src/vespa/documentapi/messagebus/messages/CMakeLists.txt @@ -1,8 +1,6 @@ # Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. vespa_add_library(documentapi_documentapimessages OBJECT SOURCES - batchdocumentupdatemessage.cpp - batchdocumentupdatereply.cpp documentignoredreply.cpp documentmessage.cpp documentreply.cpp diff --git a/documentapi/src/vespa/documentapi/messagebus/messages/batchdocumentupdatemessage.cpp b/documentapi/src/vespa/documentapi/messagebus/messages/batchdocumentupdatemessage.cpp deleted file mode 100644 index 2b38f9b24ae..00000000000 --- a/documentapi/src/vespa/documentapi/messagebus/messages/batchdocumentupdatemessage.cpp +++ /dev/null @@ -1,82 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "batchdocumentupdatemessage.h" -#include "batchdocumentupdatereply.h" -#include <vespa/documentapi/messagebus/documentprotocol.h> -#include <vespa/document/bucket/bucketidfactory.h> -#include <vespa/vespalib/util/exceptions.h> - -namespace documentapi { - -BatchDocumentUpdateMessage::BatchDocumentUpdateMessage(uint64_t userId) - : _userId(userId) -{ - setBucketId(document::UserDocIdString(vespalib::make_string("userdoc:foo:%lu:bar", _userId))); -} - -BatchDocumentUpdateMessage::BatchDocumentUpdateMessage(const string& group) - : _userId(0), - _group(group) -{ - setBucketId(document::GroupDocIdString("groupdoc:foo:" + _group + ":bar")); -} - -BatchDocumentUpdateMessage::~BatchDocumentUpdateMessage() {} - -void -BatchDocumentUpdateMessage::setBucketId(const document::IdString& idString) -{ - document::BucketIdFactory factory; - _bucketId = factory.getBucketId(document::DocumentId(idString)); -} - -void -BatchDocumentUpdateMessage::addUpdate(document::DocumentUpdate::SP update) -{ - verifyUpdate(*update); - _updates.push_back(update); -} - -void -BatchDocumentUpdateMessage::verifyUpdate(const document::DocumentUpdate& update) { - const document::IdString& idString = update.getId().getScheme(); - - if (_group.length()) { - string group; - - if (idString.hasGroup()) { - group = idString.getGroup(); - } else { - throw vespalib::IllegalArgumentException("Batch update message can only contain groupdoc or orderdoc items"); - } - - if (group != _group) { - throw vespalib::IllegalArgumentException(vespalib::make_string("Batch update message can not contain messages from group %s, only group %s", group.c_str(), _group.c_str())); - } - } else { - uint64_t userId; - - if (idString.hasNumber()) { - userId = idString.getNumber(); - } else { - throw vespalib::IllegalArgumentException("Batch update message can only contain userdoc or orderdoc items"); - } - - if (userId != _userId) { - throw vespalib::IllegalArgumentException(vespalib::make_string("Batch update message can not contain messages from user %llu, only user %llu", (long long unsigned)userId, (long long unsigned)_userId)); - } - } -} - -DocumentReply::UP -BatchDocumentUpdateMessage::doCreateReply() const -{ - return DocumentReply::UP(new BatchDocumentUpdateReply()); -} - -uint32_t -BatchDocumentUpdateMessage::getType() const { - return DocumentProtocol::MESSAGE_BATCHDOCUMENTUPDATE; -} - -} diff --git a/documentapi/src/vespa/documentapi/messagebus/messages/batchdocumentupdatemessage.h b/documentapi/src/vespa/documentapi/messagebus/messages/batchdocumentupdatemessage.h deleted file mode 100644 index 54599c2627b..00000000000 --- a/documentapi/src/vespa/documentapi/messagebus/messages/batchdocumentupdatemessage.h +++ /dev/null @@ -1,81 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#pragma once - -#include "documentmessage.h" -#include "writedocumentreply.h" -#include <vespa/document/update/documentupdate.h> -#include <vespa/document/bucket/bucketid.h> -#include <vespa/document/base/idstring.h> - -namespace documentapi { - -/** - Message to use to send multiple updates for documents - belonging to the same user or group to Vespa. Using this - message improves performance in VDS mainly. -*/ -class BatchDocumentUpdateMessage : public DocumentMessage -{ -public: - typedef std::vector<document::DocumentUpdate::SP > UpdateList; - - /** - Creates a batch update message that can contain only updates - for documents belonging to the given user. - */ - BatchDocumentUpdateMessage(uint64_t userId); - - /** - Creates a batch update message that can contain only updates - for documents belonging to the given group. - */ - BatchDocumentUpdateMessage(const string& group); - ~BatchDocumentUpdateMessage(); - - /** - @return Returns a list of the updates to be performed. - */ - const UpdateList& getUpdates() const { return _updates; }; - - /** - Adds an update to be performed. - */ - void addUpdate(document::DocumentUpdate::SP update); - - /** - Returns the user id that this batch can contain. - Only valid if this object was created with the first constructor. - */ - uint64_t getUserId() const { return _userId; }; - - /** - Returns the grouo that this batch can contain. - Only valid if this object was created with the second constructor. - */ - const string& getGroup() const { return _group; } - - uint32_t getType() const override; - - /** - Returns a bucket id suitable for routing this message. - */ - const document::BucketId& getBucketId() const { return _bucketId; } - - string toString() const override { return "batchdocumentupdatemessage"; } - -protected: - DocumentReply::UP doCreateReply() const override; - -private: - uint64_t _userId; - string _group; - - UpdateList _updates; - document::BucketId _bucketId; - - void verifyUpdate(const document::DocumentUpdate& update); - void setBucketId(const document::IdString& idString); -}; - -} - diff --git a/documentapi/src/vespa/documentapi/messagebus/messages/batchdocumentupdatereply.cpp b/documentapi/src/vespa/documentapi/messagebus/messages/batchdocumentupdatereply.cpp deleted file mode 100644 index 483740233c0..00000000000 --- a/documentapi/src/vespa/documentapi/messagebus/messages/batchdocumentupdatereply.cpp +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "batchdocumentupdatereply.h" -#include <vespa/documentapi/messagebus/documentprotocol.h> - -namespace documentapi { - -BatchDocumentUpdateReply::BatchDocumentUpdateReply() - : WriteDocumentReply(DocumentProtocol::REPLY_BATCHDOCUMENTUPDATE) -{ } - -BatchDocumentUpdateReply::~BatchDocumentUpdateReply() {} - -} diff --git a/documentapi/src/vespa/documentapi/messagebus/messages/batchdocumentupdatereply.h b/documentapi/src/vespa/documentapi/messagebus/messages/batchdocumentupdatereply.h deleted file mode 100644 index be1f5bc75d5..00000000000 --- a/documentapi/src/vespa/documentapi/messagebus/messages/batchdocumentupdatereply.h +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#pragma once - -#include "writedocumentreply.h" - -namespace documentapi { - -class BatchDocumentUpdateReply : public WriteDocumentReply -{ - /** - * If all documents to update are found, this vector will be empty. If - * one or more documents are not found, this vector will have the size of - * the initial number of updates, with entries set to true where the - * corresponding update was not found. - */ - std::vector<bool> _documentsNotFound; -public: - typedef std::unique_ptr<BatchDocumentUpdateReply> UP; - typedef std::shared_ptr<BatchDocumentUpdateReply> SP; - - BatchDocumentUpdateReply(); - ~BatchDocumentUpdateReply(); - - const std::vector<bool>& getDocumentsNotFound() const { return _documentsNotFound; } - std::vector<bool>& getDocumentsNotFound() { return _documentsNotFound; } - - string toString() const override { return "batchdocumentupdatereply"; } -}; - -} diff --git a/documentapi/src/vespa/documentapi/messagebus/policies/documentrouteselectorpolicy.cpp b/documentapi/src/vespa/documentapi/messagebus/policies/documentrouteselectorpolicy.cpp index 42120b8052a..62135150d98 100644 --- a/documentapi/src/vespa/documentapi/messagebus/policies/documentrouteselectorpolicy.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/policies/documentrouteselectorpolicy.cpp @@ -4,7 +4,6 @@ #include <vespa/document/bucket/bucketidfactory.h> #include <vespa/document/select/parser.h> #include <vespa/documentapi/messagebus/documentprotocol.h> -#include <vespa/documentapi/messagebus/messages/batchdocumentupdatemessage.h> #include <vespa/documentapi/messagebus/messages/putdocumentmessage.h> #include <vespa/documentapi/messagebus/messages/updatedocumentmessage.h> #include <vespa/documentapi/messagebus/messages/documentignoredreply.h> @@ -138,17 +137,6 @@ DocumentRouteSelectorPolicy::select(mbus::RoutingContext &context, const vespali return true; } } - - case DocumentProtocol::MESSAGE_BATCHDOCUMENTUPDATE: - { - const BatchDocumentUpdateMessage& mom = static_cast<const BatchDocumentUpdateMessage&>(msg); - for (uint32_t i = 0; i < mom.getUpdates().size(); i++) { - if (it->second->contains(*mom.getUpdates()[i]) == Result::False) { - return false; - } - } - return true; - } default: return true; } diff --git a/documentapi/src/vespa/documentapi/messagebus/policies/storagepolicy.cpp b/documentapi/src/vespa/documentapi/messagebus/policies/storagepolicy.cpp index 723c6fc836a..472440d7863 100644 --- a/documentapi/src/vespa/documentapi/messagebus/policies/storagepolicy.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/policies/storagepolicy.cpp @@ -2,6 +2,7 @@ #include "storagepolicy.h" #include <vespa/document/base/documentid.h> +#include <vespa/document/update/documentupdate.h> #include <vespa/messagebus/emptyreply.h> #include <vespa/messagebus/routing/verbatimdirective.h> #include <vespa/documentapi/documentapi.h> @@ -150,10 +151,6 @@ StoragePolicy::doSelect(mbus::RoutingContext &context) id = static_cast<const RemoveLocationMessage&>(msg).getBucketId(); break; - case DocumentProtocol::MESSAGE_BATCHDOCUMENTUPDATE: - id = static_cast<const BatchDocumentUpdateMessage&>(msg).getBucketId(); - break; - default: LOG(error, "Message type '%d' not supported.", msg.getType()); return; diff --git a/documentapi/src/vespa/documentapi/messagebus/routablefactories50.cpp b/documentapi/src/vespa/documentapi/messagebus/routablefactories50.cpp index 26d85b57522..120ee1facd0 100644 --- a/documentapi/src/vespa/documentapi/messagebus/routablefactories50.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/routablefactories50.cpp @@ -4,6 +4,7 @@ #include <vespa/document/bucket/bucketidfactory.h> #include <vespa/document/bucket/fixed_bucket_spaces.h> #include <vespa/document/select/parser.h> +#include <vespa/document/update/documentupdate.h> #include <vespa/documentapi/documentapi.h> #include <vespa/documentapi/loadtypes/loadtypeset.h> #include <vespa/vespalib/objects/nbostream.h> @@ -67,69 +68,6 @@ RoutableFactories50::DocumentReplyFactory::decode(document::ByteBuffer &in, cons //////////////////////////////////////////////////////////////////////////////// DocumentMessage::UP -RoutableFactories50::BatchDocumentUpdateMessageFactory::doDecode(document::ByteBuffer &buf) const -{ - uint64_t userId = (uint64_t)decodeLong(buf); - string group = decodeString(buf); - - auto msg = (group.length()) - ? std::make_unique<BatchDocumentUpdateMessage>(group) - : std::make_unique<BatchDocumentUpdateMessage>(userId); - - uint32_t len = decodeInt(buf); - for (uint32_t i = 0; i < len; i++) { - document::DocumentUpdate::SP upd = document::DocumentUpdate::createHEAD(_repo, buf); - msg->addUpdate(upd); - } - - return msg; -} - -bool -RoutableFactories50::BatchDocumentUpdateMessageFactory::doEncode(const DocumentMessage &obj, vespalib::GrowableByteBuffer &buf) const -{ - const BatchDocumentUpdateMessage &msg = static_cast<const BatchDocumentUpdateMessage&>(obj); - - buf.putLong(msg.getUserId()); - buf.putString(msg.getGroup()); - buf.putInt(msg.getUpdates().size()); - - vespalib::nbostream stream; - for (const auto & update : msg.getUpdates()) { - update->serializeHEAD(stream); - } - buf.putBytes(stream.c_str(), stream.size()); - - return true; -} - -DocumentReply::UP -RoutableFactories50::BatchDocumentUpdateReplyFactory::doDecode(document::ByteBuffer &buf) const -{ - auto reply = std::make_unique<BatchDocumentUpdateReply>(); - reply->setHighestModificationTimestamp(decodeLong(buf)); - std::vector<bool>& notFound = reply->getDocumentsNotFound(); - notFound.resize(decodeInt(buf)); - for (std::size_t i = 0; i < notFound.size(); ++i) { - notFound[i] = decodeBoolean(buf); - } - return reply; -} - -bool -RoutableFactories50::BatchDocumentUpdateReplyFactory::doEncode(const DocumentReply &obj, vespalib::GrowableByteBuffer &buf) const -{ - const BatchDocumentUpdateReply& reply = static_cast<const BatchDocumentUpdateReply&>(obj); - buf.putLong(reply.getHighestModificationTimestamp()); - const std::vector<bool>& notFoundV = reply.getDocumentsNotFound(); - buf.putInt(notFoundV.size()); - for (bool notFound : notFoundV) { - buf.putBoolean(notFound); - } - return true; -} - -DocumentMessage::UP RoutableFactories50::CreateVisitorMessageFactory::doDecode(document::ByteBuffer &buf) const { auto msg = std::make_unique<CreateVisitorMessage>(); diff --git a/documentapi/src/vespa/documentapi/messagebus/routablefactories50.h b/documentapi/src/vespa/documentapi/messagebus/routablefactories50.h index 12d5c560786..f96b9641800 100644 --- a/documentapi/src/vespa/documentapi/messagebus/routablefactories50.h +++ b/documentapi/src/vespa/documentapi/messagebus/routablefactories50.h @@ -157,19 +157,6 @@ public: // Factories // //////////////////////////////////////////////////////////////////////////////// - - class BatchDocumentUpdateMessageFactory : public DocumentMessageFactory { - const document::DocumentTypeRepo &_repo; - DocumentMessage::UP doDecode(document::ByteBuffer &buf) const override; - bool doEncode(const DocumentMessage &msg, vespalib::GrowableByteBuffer &buf) const override; - public: - BatchDocumentUpdateMessageFactory(const document::DocumentTypeRepo &r) : _repo(r) {} - }; - class BatchDocumentUpdateReplyFactory : public DocumentReplyFactory { - protected: - DocumentReply::UP doDecode(document::ByteBuffer &buf) const override; - bool doEncode(const DocumentReply &reply, vespalib::GrowableByteBuffer &buf) const override; - }; class CreateVisitorMessageFactory : public DocumentMessageFactory { const document::DocumentTypeRepo &_repo; protected: diff --git a/documentapi/test/crosslanguagefiles/5-cpp-BatchDocumentUpdateMessage.dat b/documentapi/test/crosslanguagefiles/5-cpp-BatchDocumentUpdateMessage.dat Binary files differdeleted file mode 100644 index 95e663088c3..00000000000 --- a/documentapi/test/crosslanguagefiles/5-cpp-BatchDocumentUpdateMessage.dat +++ /dev/null diff --git a/documentapi/test/crosslanguagefiles/5-cpp-BatchDocumentUpdateReply.dat b/documentapi/test/crosslanguagefiles/5-cpp-BatchDocumentUpdateReply.dat Binary files differdeleted file mode 100644 index 216db17f80e..00000000000 --- a/documentapi/test/crosslanguagefiles/5-cpp-BatchDocumentUpdateReply.dat +++ /dev/null diff --git a/documentapi/test/crosslanguagefiles/5-java-BatchDocumentUpdateMessage.dat b/documentapi/test/crosslanguagefiles/5-java-BatchDocumentUpdateMessage.dat Binary files differdeleted file mode 100644 index 95e663088c3..00000000000 --- a/documentapi/test/crosslanguagefiles/5-java-BatchDocumentUpdateMessage.dat +++ /dev/null diff --git a/documentapi/test/crosslanguagefiles/5-java-BatchDocumentUpdateReply.dat b/documentapi/test/crosslanguagefiles/5-java-BatchDocumentUpdateReply.dat Binary files differdeleted file mode 100644 index 216db17f80e..00000000000 --- a/documentapi/test/crosslanguagefiles/5-java-BatchDocumentUpdateReply.dat +++ /dev/null diff --git a/documentapi/test/crosslanguagefiles/5.1-cpp-BatchDocumentUpdateMessage.dat b/documentapi/test/crosslanguagefiles/5.1-cpp-BatchDocumentUpdateMessage.dat Binary files differdeleted file mode 100644 index 95e663088c3..00000000000 --- a/documentapi/test/crosslanguagefiles/5.1-cpp-BatchDocumentUpdateMessage.dat +++ /dev/null diff --git a/documentapi/test/crosslanguagefiles/5.1-cpp-BatchDocumentUpdateReply.dat b/documentapi/test/crosslanguagefiles/5.1-cpp-BatchDocumentUpdateReply.dat Binary files differdeleted file mode 100644 index 216db17f80e..00000000000 --- a/documentapi/test/crosslanguagefiles/5.1-cpp-BatchDocumentUpdateReply.dat +++ /dev/null diff --git a/documentapi/test/crosslanguagefiles/5.1-java-BatchDocumentUpdateMessage.dat b/documentapi/test/crosslanguagefiles/5.1-java-BatchDocumentUpdateMessage.dat Binary files differdeleted file mode 100644 index 95e663088c3..00000000000 --- a/documentapi/test/crosslanguagefiles/5.1-java-BatchDocumentUpdateMessage.dat +++ /dev/null diff --git a/documentapi/test/crosslanguagefiles/5.1-java-BatchDocumentUpdateReply.dat b/documentapi/test/crosslanguagefiles/5.1-java-BatchDocumentUpdateReply.dat Binary files differdeleted file mode 100644 index 216db17f80e..00000000000 --- a/documentapi/test/crosslanguagefiles/5.1-java-BatchDocumentUpdateReply.dat +++ /dev/null diff --git a/documentapi/test/crosslanguagefiles/5.115-cpp-BatchDocumentUpdateMessage.dat b/documentapi/test/crosslanguagefiles/5.115-cpp-BatchDocumentUpdateMessage.dat Binary files differdeleted file mode 100644 index 95e663088c3..00000000000 --- a/documentapi/test/crosslanguagefiles/5.115-cpp-BatchDocumentUpdateMessage.dat +++ /dev/null diff --git a/documentapi/test/crosslanguagefiles/5.115-cpp-BatchDocumentUpdateReply.dat b/documentapi/test/crosslanguagefiles/5.115-cpp-BatchDocumentUpdateReply.dat Binary files differdeleted file mode 100644 index 216db17f80e..00000000000 --- a/documentapi/test/crosslanguagefiles/5.115-cpp-BatchDocumentUpdateReply.dat +++ /dev/null diff --git a/documentapi/test/crosslanguagefiles/5.115-java-BatchDocumentUpdateMessage.dat b/documentapi/test/crosslanguagefiles/5.115-java-BatchDocumentUpdateMessage.dat Binary files differdeleted file mode 100644 index 95e663088c3..00000000000 --- a/documentapi/test/crosslanguagefiles/5.115-java-BatchDocumentUpdateMessage.dat +++ /dev/null diff --git a/documentapi/test/crosslanguagefiles/5.115-java-BatchDocumentUpdateReply.dat b/documentapi/test/crosslanguagefiles/5.115-java-BatchDocumentUpdateReply.dat Binary files differdeleted file mode 100644 index 216db17f80e..00000000000 --- a/documentapi/test/crosslanguagefiles/5.115-java-BatchDocumentUpdateReply.dat +++ /dev/null diff --git a/documentapi/test/crosslanguagefiles/6.221-cpp-BatchDocumentUpdateMessage.dat b/documentapi/test/crosslanguagefiles/6.221-cpp-BatchDocumentUpdateMessage.dat Binary files differdeleted file mode 100644 index 95e663088c3..00000000000 --- a/documentapi/test/crosslanguagefiles/6.221-cpp-BatchDocumentUpdateMessage.dat +++ /dev/null diff --git a/documentapi/test/crosslanguagefiles/6.221-cpp-BatchDocumentUpdateReply.dat b/documentapi/test/crosslanguagefiles/6.221-cpp-BatchDocumentUpdateReply.dat Binary files differdeleted file mode 100644 index 216db17f80e..00000000000 --- a/documentapi/test/crosslanguagefiles/6.221-cpp-BatchDocumentUpdateReply.dat +++ /dev/null diff --git a/documentapi/test/crosslanguagefiles/6.221-cpp-DocumentSummaryMessage-1.dat b/documentapi/test/crosslanguagefiles/6.221-cpp-DocumentSummaryMessage-1.dat Binary files differnew file mode 100644 index 00000000000..0107dd5f350 --- /dev/null +++ b/documentapi/test/crosslanguagefiles/6.221-cpp-DocumentSummaryMessage-1.dat diff --git a/documentapi/test/crosslanguagefiles/6.221-cpp-DocumentSummaryMessage-2.dat b/documentapi/test/crosslanguagefiles/6.221-cpp-DocumentSummaryMessage-2.dat Binary files differnew file mode 100644 index 00000000000..57187093f28 --- /dev/null +++ b/documentapi/test/crosslanguagefiles/6.221-cpp-DocumentSummaryMessage-2.dat diff --git a/documentapi/test/crosslanguagefiles/6.221-cpp-DocumentSummaryMessage-3.dat b/documentapi/test/crosslanguagefiles/6.221-cpp-DocumentSummaryMessage-3.dat Binary files differnew file mode 100644 index 00000000000..6a516d38d17 --- /dev/null +++ b/documentapi/test/crosslanguagefiles/6.221-cpp-DocumentSummaryMessage-3.dat diff --git a/documentapi/test/crosslanguagefiles/6.221-cpp-QueryResultMessage-1.dat b/documentapi/test/crosslanguagefiles/6.221-cpp-QueryResultMessage-1.dat Binary files differnew file mode 100644 index 00000000000..dbf830c9365 --- /dev/null +++ b/documentapi/test/crosslanguagefiles/6.221-cpp-QueryResultMessage-1.dat diff --git a/documentapi/test/crosslanguagefiles/6.221-cpp-QueryResultMessage-2.dat b/documentapi/test/crosslanguagefiles/6.221-cpp-QueryResultMessage-2.dat Binary files differnew file mode 100644 index 00000000000..094143cf78d --- /dev/null +++ b/documentapi/test/crosslanguagefiles/6.221-cpp-QueryResultMessage-2.dat diff --git a/documentapi/test/crosslanguagefiles/6.221-cpp-QueryResultMessage-3.dat b/documentapi/test/crosslanguagefiles/6.221-cpp-QueryResultMessage-3.dat Binary files differnew file mode 100644 index 00000000000..3341d74052b --- /dev/null +++ b/documentapi/test/crosslanguagefiles/6.221-cpp-QueryResultMessage-3.dat diff --git a/documentapi/test/crosslanguagefiles/6.221-cpp-QueryResultMessage-4.dat b/documentapi/test/crosslanguagefiles/6.221-cpp-QueryResultMessage-4.dat Binary files differnew file mode 100644 index 00000000000..8aaaefff491 --- /dev/null +++ b/documentapi/test/crosslanguagefiles/6.221-cpp-QueryResultMessage-4.dat diff --git a/documentapi/test/crosslanguagefiles/6.221-cpp-QueryResultMessage-5.dat b/documentapi/test/crosslanguagefiles/6.221-cpp-QueryResultMessage-5.dat Binary files differnew file mode 100644 index 00000000000..e66ed1f07d4 --- /dev/null +++ b/documentapi/test/crosslanguagefiles/6.221-cpp-QueryResultMessage-5.dat diff --git a/documentapi/test/crosslanguagefiles/6.221-cpp-SearchResultMessage-1.dat b/documentapi/test/crosslanguagefiles/6.221-cpp-SearchResultMessage-1.dat Binary files differnew file mode 100644 index 00000000000..988f9fdab1f --- /dev/null +++ b/documentapi/test/crosslanguagefiles/6.221-cpp-SearchResultMessage-1.dat diff --git a/documentapi/test/crosslanguagefiles/6.221-cpp-SearchResultMessage-2.dat b/documentapi/test/crosslanguagefiles/6.221-cpp-SearchResultMessage-2.dat Binary files differnew file mode 100644 index 00000000000..ac277d09643 --- /dev/null +++ b/documentapi/test/crosslanguagefiles/6.221-cpp-SearchResultMessage-2.dat diff --git a/documentapi/test/crosslanguagefiles/6.221-cpp-SearchResultMessage-3.dat b/documentapi/test/crosslanguagefiles/6.221-cpp-SearchResultMessage-3.dat Binary files differnew file mode 100644 index 00000000000..03b49c8a0ac --- /dev/null +++ b/documentapi/test/crosslanguagefiles/6.221-cpp-SearchResultMessage-3.dat diff --git a/documentapi/test/crosslanguagefiles/6.221-cpp-SearchResultMessage-4.dat b/documentapi/test/crosslanguagefiles/6.221-cpp-SearchResultMessage-4.dat Binary files differnew file mode 100644 index 00000000000..d52e574ea44 --- /dev/null +++ b/documentapi/test/crosslanguagefiles/6.221-cpp-SearchResultMessage-4.dat diff --git a/documentapi/test/crosslanguagefiles/6.221-cpp-SearchResultMessage-5.dat b/documentapi/test/crosslanguagefiles/6.221-cpp-SearchResultMessage-5.dat Binary files differnew file mode 100644 index 00000000000..e68654e9941 --- /dev/null +++ b/documentapi/test/crosslanguagefiles/6.221-cpp-SearchResultMessage-5.dat diff --git a/documentapi/test/crosslanguagefiles/6.221-java-BatchDocumentUpdateMessage.dat b/documentapi/test/crosslanguagefiles/6.221-java-BatchDocumentUpdateMessage.dat Binary files differdeleted file mode 100644 index 95e663088c3..00000000000 --- a/documentapi/test/crosslanguagefiles/6.221-java-BatchDocumentUpdateMessage.dat +++ /dev/null diff --git a/documentapi/test/crosslanguagefiles/6.221-java-BatchDocumentUpdateReply.dat b/documentapi/test/crosslanguagefiles/6.221-java-BatchDocumentUpdateReply.dat Binary files differdeleted file mode 100644 index 216db17f80e..00000000000 --- a/documentapi/test/crosslanguagefiles/6.221-java-BatchDocumentUpdateReply.dat +++ /dev/null diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/SimpleAdapterFactory.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/SimpleAdapterFactory.java index 252f6c5bd12..43ccd6d48a8 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/SimpleAdapterFactory.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/SimpleAdapterFactory.java @@ -48,23 +48,31 @@ public class SimpleAdapterFactory implements AdapterFactory { DocumentId docId = upd.getId(); Document complete = new Document(docType, upd.getId()); for (FieldPathUpdate fieldUpd : upd) { - if (FieldPathUpdateHelper.isComplete(fieldUpd)) { - // A 'complete' field path update is basically a regular top-level field update - // in wolf's clothing. Convert it to a regular field update to be friendlier - // towards the search core backend. - FieldPathUpdateHelper.applyUpdate(fieldUpd, complete); - } else { - ret.add(new IdentityFieldPathUpdateAdapter(fieldUpd, newDocumentAdapter(complete, true))); + try { + if (FieldPathUpdateHelper.isComplete(fieldUpd)) { + // A 'complete' field path update is basically a regular top-level field update + // in wolf's clothing. Convert it to a regular field update to be friendlier + // towards the search core backend. + FieldPathUpdateHelper.applyUpdate(fieldUpd, complete); + } else { + ret.add(new IdentityFieldPathUpdateAdapter(fieldUpd, newDocumentAdapter(complete, true))); + } + } catch (NullPointerException e) { + throw new IllegalArgumentException("Exception during handling of update '" + fieldUpd + "' to field '" + fieldUpd.getFieldPath() + "'", e); } } for (FieldUpdate fieldUpd : upd.getFieldUpdates()) { Field field = fieldUpd.getField(); for (ValueUpdate valueUpd : fieldUpd.getValueUpdates()) { - if (FieldUpdateHelper.isComplete(field, valueUpd)) { - FieldUpdateHelper.applyUpdate(field, valueUpd, complete); - } else { - Document partial = FieldUpdateHelper.newPartialDocument(docType, docId, field, valueUpd); - ret.add(FieldUpdateAdapter.fromPartialUpdate(expressionSelector.selectExpression(docType, field.getName()),newDocumentAdapter(partial, true), valueUpd)); + try { + if (FieldUpdateHelper.isComplete(field, valueUpd)) { + FieldUpdateHelper.applyUpdate(field, valueUpd, complete); + } else { + Document partial = FieldUpdateHelper.newPartialDocument(docType, docId, field, valueUpd); + ret.add(FieldUpdateAdapter.fromPartialUpdate(expressionSelector.selectExpression(docType, field.getName()), newDocumentAdapter(partial, true), valueUpd)); + } + } catch (NullPointerException e) { + throw new IllegalArgumentException("Exception during handling of update '" + valueUpd + "' to field '" + field + "'", e); } } } diff --git a/jrt/src/com/yahoo/jrt/MaybeTlsCryptoEngine.java b/jrt/src/com/yahoo/jrt/MaybeTlsCryptoEngine.java index 8cb560246e8..6664934799c 100644 --- a/jrt/src/com/yahoo/jrt/MaybeTlsCryptoEngine.java +++ b/jrt/src/com/yahoo/jrt/MaybeTlsCryptoEngine.java @@ -7,28 +7,28 @@ import java.nio.channels.SocketChannel; * A crypto engine that supports both tls encrypted connections and * unencrypted connections. The use of tls for incoming connections is * auto-detected using clever heuristics. The use of tls for outgoing - * connections is controlled by the useTls flag given to the + * connections is controlled by the useTlsWhenClient flag given to the * constructor. **/ public class MaybeTlsCryptoEngine implements CryptoEngine { private final TlsCryptoEngine tlsEngine; - private final boolean useTls; + private final boolean useTlsWhenClient; - public MaybeTlsCryptoEngine(TlsCryptoEngine tlsEngine, boolean useTls) { + public MaybeTlsCryptoEngine(TlsCryptoEngine tlsEngine, boolean useTlsWhenClient) { this.tlsEngine = tlsEngine; - this.useTls = useTls; + this.useTlsWhenClient = useTlsWhenClient; } @Override public CryptoSocket createCryptoSocket(SocketChannel channel, boolean isServer) { if (isServer) { return new MaybeTlsCryptoSocket(channel, tlsEngine); - } else if (useTls) { + } else if (useTlsWhenClient) { return tlsEngine.createCryptoSocket(channel, false); } else { return new NullCryptoSocket(channel); } } - @Override public String toString() { return "MaybeTlsCryptoEngine(useTls:" + useTls + ")"; } + @Override public String toString() { return "MaybeTlsCryptoEngine(useTlsWhenClient:" + useTlsWhenClient + ")"; } } diff --git a/jrt/src/com/yahoo/jrt/MaybeTlsCryptoSocket.java b/jrt/src/com/yahoo/jrt/MaybeTlsCryptoSocket.java index 7cedbcda9a1..1cf3dfd1261 100644 --- a/jrt/src/com/yahoo/jrt/MaybeTlsCryptoSocket.java +++ b/jrt/src/com/yahoo/jrt/MaybeTlsCryptoSocket.java @@ -16,13 +16,13 @@ import java.nio.channels.SocketChannel; **/ public class MaybeTlsCryptoSocket implements CryptoSocket { - private static final int snoop_size = 9; + private static final int SNOOP_SIZE = 9; private CryptoSocket socket; // 'data' is the first 9 bytes received from the client public static boolean looksLikeTlsToMe(byte[] data) { - if (data.length != snoop_size) { + if (data.length != SNOOP_SIZE) { return false; // wrong data size for tls detection } if (data[0] != 22) { @@ -67,13 +67,13 @@ public class MaybeTlsCryptoSocket implements CryptoSocket { @Override public HandshakeResult handshake() throws IOException { if (factory != null) { - channel().read(buffer.getWritable(snoop_size)); - if (buffer.bytes() < snoop_size) { + channel().read(buffer.getWritable(SNOOP_SIZE)); + if (buffer.bytes() < SNOOP_SIZE) { return HandshakeResult.NEED_READ; } - byte[] data = new byte[snoop_size]; + byte[] data = new byte[SNOOP_SIZE]; ByteBuffer src = buffer.getReadable(); - for (int i = 0; i < snoop_size; i++) { + for (int i = 0; i < SNOOP_SIZE; i++) { data[i] = src.get(i); } if (looksLikeTlsToMe(data)) { diff --git a/jrt/tests/com/yahoo/jrt/TlsDetectionTest.java b/jrt/tests/com/yahoo/jrt/TlsDetectionTest.java index 9bd37e25772..47a8a20deab 100644 --- a/jrt/tests/com/yahoo/jrt/TlsDetectionTest.java +++ b/jrt/tests/com/yahoo/jrt/TlsDetectionTest.java @@ -1,20 +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.jrt; +import static org.junit.Assert.assertEquals; + public class TlsDetectionTest { - static private String message(byte[] data, boolean actual) { - String msg = "["; + static private String message(byte[] data) { + String msg = "isTls(["; String delimiter = ""; for (byte b: data) { msg += delimiter + (b & 0xff); delimiter = ", "; } - if (actual) { - msg += "] wrongfully detected as tls"; - } else { - msg += "] wrongfully rejected as not tls"; - } + msg += "])"; return msg; } @@ -23,10 +21,7 @@ public class TlsDetectionTest { for (int i = 0; i < data.length; i++) { data[i] = (byte) values[i]; } - boolean actual = MaybeTlsCryptoSocket.looksLikeTlsToMe(data); - if(actual != expect) { - throw new AssertionError(message(data, actual)); - } + assertEquals(message(data), expect, MaybeTlsCryptoSocket.looksLikeTlsToMe(data)); } @org.junit.Test public void testValidHandshake() { diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java index 4a19e5fe215..cdfefa6a184 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperations.java @@ -38,7 +38,7 @@ public interface DockerOperations { /** * Try to suspend node. Suspending a node means the node should be taken offline, - * such that maintenance can be done of the node (upgrading, rebooting, etc), + * such that maintenance of the node can be done (upgrading, rebooting, etc), * and such that we will start serving again as soon as possible afterwards. */ void trySuspendNode(ContainerName containerName); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java index ba8a2e55587..12c1b9bcf11 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImpl.java @@ -145,7 +145,10 @@ public class NodeAdminImpl implements NodeAdmin { // Each container may spend 1-1:30 minutes stopping nodeAgentsByHostname.values().parallelStream() .filter(nodeAgent -> hostnames.contains(nodeAgent.getHostname())) - .forEach(NodeAgent::stopServices); + .forEach(nodeAgent -> { + nodeAgent.suspend(); + nodeAgent.stopServices(); + }); } public int getNumberOfNodeAgents() { diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgent.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgent.java index 92c44969d5e..9b759b208eb 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgent.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgent.java @@ -18,9 +18,18 @@ public interface NodeAgent { */ boolean setFrozen(boolean frozen); + /** + * Stop services running on node. Depending on the state of the node, {@link #suspend()} might need to be + * called before calling this method. + */ void stopServices(); /** + * Suspend node. Take node offline (e.g. take node out of VIP, drain traffic, prepare for restart etc.) + */ + void suspend(); + + /** * Returns a map containing all relevant NodeAgent variables and their current values. */ Map<String, Object> debugInfo(); 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 ad38306547d..43e3051ce84 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 @@ -302,13 +302,23 @@ public class NodeAgentImpl implements NodeAgent { logger.info("Stopping services"); if (containerState == ABSENT) return; try { - dockerOperations.trySuspendNode(containerName); dockerOperations.stopServicesOnNode(containerName); } catch (ContainerNotFoundException e) { containerState = ABSENT; } } + @Override + public void suspend() { + logger.info("Suspending services on node"); + if (containerState == ABSENT) return; + try { + dockerOperations.trySuspendNode(containerName); + } catch (ContainerNotFoundException e) { + containerState = ABSENT; + } + } + private Optional<String> shouldRemoveContainer(NodeSpec node, Container existingContainer) { final Node.State nodeState = node.getState(); if (nodeState == Node.State.dirty || nodeState == Node.State.provisioned) { @@ -344,6 +354,9 @@ public class NodeAgentImpl implements NodeAgent { } try { + if (node.getState() != Node.State.dirty) { + suspend(); + } stopServices(); } catch (Exception e) { logger.info("Failed stopping services, ignoring", e); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java index e0031f9b9b3..28a2b8ff5e0 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java @@ -426,6 +426,8 @@ public class NodeAgentImplTest { verify(dockerOperations, never()).createContainer(eq(containerName), any(), any()); verify(dockerOperations, never()).startContainer(eq(containerName)); + verify(dockerOperations, never()).trySuspendNode(eq(containerName)); + verify(dockerOperations, times(1)).stopServicesOnNode(eq(containerName)); verify(orchestrator, never()).resume(any(String.class)); verify(orchestrator, never()).suspend(any(String.class)); // current Docker image and vespa version should be cleared diff --git a/parent/pom.xml b/parent/pom.xml index 96075974567..a46b86f2725 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -461,7 +461,7 @@ <artifactId>commons-pool</artifactId> <version>1.5.6</version> </dependency> - <!-- Explicitly included to get Zookeeper version 3.4.12, + <!-- Explicitly included to get Zookeeper version 3.4.13, can be excluded if you want the Zookeeper version used by curator by default --> @@ -673,6 +673,11 @@ <artifactId>language-detector</artifactId> <version>0.6</version> </dependency> + <dependency> + <groupId>net.java.dev.jna</groupId> + <artifactId>jna</artifactId> + <version>4.5.2</version> + </dependency> </dependencies> </dependencyManagement> diff --git a/storage/src/tests/distributor/twophaseupdateoperationtest.cpp b/storage/src/tests/distributor/twophaseupdateoperationtest.cpp index ea2cc00c642..f4b80f7961c 100644 --- a/storage/src/tests/distributor/twophaseupdateoperationtest.cpp +++ b/storage/src/tests/distributor/twophaseupdateoperationtest.cpp @@ -11,7 +11,6 @@ #include <vespa/storage/distributor/externaloperationhandler.h> #include <vespa/storageapi/message/persistence.h> #include <vespa/storage/distributor/operations/external/twophaseupdateoperation.h> -#include <vespa/storageapi/message/batch.h> #include <tests/distributor/distributortestutil.h> #include <vespa/document/test/make_document_bucket.h> #include <vespa/storage/distributor/distributor.h> diff --git a/storage/src/tests/persistence/filestorage/filestormanagertest.cpp b/storage/src/tests/persistence/filestorage/filestormanagertest.cpp index 369e820f987..61c55248370 100644 --- a/storage/src/tests/persistence/filestorage/filestormanagertest.cpp +++ b/storage/src/tests/persistence/filestorage/filestormanagertest.cpp @@ -12,12 +12,12 @@ #include <vespa/storage/persistence/filestorage/filestormanager.h> #include <vespa/storage/persistence/filestorage/modifiedbucketchecker.h> #include <vespa/document/update/assignvalueupdate.h> +#include <vespa/document/update/documentupdate.h> #include <vespa/document/select/parser.h> #include <vespa/vdslib/state/random.h> #include <vespa/storageapi/message/bucketsplitting.h> #include <vespa/persistence/dummyimpl/dummypersistence.h> #include <vespa/persistence/spi/test.h> -#include <vespa/storageapi/message/batch.h> #include <vespa/config/common/exceptions.h> #include <vespa/fastos/file.h> diff --git a/storage/src/tests/storageserver/documentapiconvertertest.cpp b/storage/src/tests/storageserver/documentapiconvertertest.cpp index 40d561bd589..7f8b1b8f34a 100644 --- a/storage/src/tests/storageserver/documentapiconvertertest.cpp +++ b/storage/src/tests/storageserver/documentapiconvertertest.cpp @@ -8,11 +8,11 @@ #include <vespa/document/repo/documenttyperepo.h> #include <vespa/document/select/parser.h> #include <vespa/document/test/make_document_bucket.h> +#include <vespa/document/update/documentupdate.h> #include <vespa/documentapi/documentapi.h> #include <vespa/messagebus/emptyreply.h> #include <vespa/storage/common/bucket_resolver.h> #include <vespa/storage/storageserver/documentapiconverter.h> -#include <vespa/storageapi/message/batch.h> #include <vespa/storageapi/message/datagram.h> #include <vespa/storageapi/message/persistence.h> #include <vespa/storageapi/message/removelocation.h> @@ -115,7 +115,6 @@ struct DocumentApiConverterTest : public CppUnit::TestFixture void testCreateVisitorReplyLastBucket(); void testDestroyVisitor(); void testVisitorInfo(); - void testBatchDocumentUpdate(); void testStatBucket(); void testGetBucketList(); void testRemoveLocation(); @@ -133,7 +132,6 @@ struct DocumentApiConverterTest : public CppUnit::TestFixture CPPUNIT_TEST(testCreateVisitorReplyLastBucket); CPPUNIT_TEST(testDestroyVisitor); CPPUNIT_TEST(testVisitorInfo); - CPPUNIT_TEST(testBatchDocumentUpdate); CPPUNIT_TEST(testStatBucket); CPPUNIT_TEST(testGetBucketList); CPPUNIT_TEST(testRemoveLocation); @@ -321,58 +319,6 @@ DocumentApiConverterTest::testVisitorInfo() } void -DocumentApiConverterTest::testBatchDocumentUpdate() -{ - std::vector<document::DocumentUpdate::SP > updates; - - { - document::DocumentId docId(document::UserDocIdString("userdoc:test:1234:test1")); - auto update = std::make_shared<document::DocumentUpdate>(*_repo, _html_type, docId); - updates.push_back(update); - } - - { - document::DocumentId docId(document::UserDocIdString("userdoc:test:1234:test2")); - auto update = std::make_shared<document::DocumentUpdate>(*_repo, _html_type, docId); - updates.push_back(update); - } - - { - document::DocumentId docId(document::UserDocIdString("userdoc:test:1234:test3")); - auto update = std::make_shared<document::DocumentUpdate>(*_repo, _html_type, docId); - updates.push_back(update); - } - - auto msg = std::make_shared<documentapi::BatchDocumentUpdateMessage>(1234); - for (std::size_t i = 0; i < updates.size(); ++i) { - msg->addUpdate(updates[i]); - } - - auto batchCmd = toStorageAPI<api::BatchDocumentUpdateCommand>(*msg); - CPPUNIT_ASSERT_EQUAL(updates.size(), batchCmd->getUpdates().size()); - for (std::size_t i = 0; i < updates.size(); ++i) { - CPPUNIT_ASSERT_EQUAL(*updates[i], *batchCmd->getUpdates()[i]); - } - - api::BatchDocumentUpdateReply batchReply(*batchCmd); - batchReply.getDocumentsNotFound().resize(3); - batchReply.getDocumentsNotFound()[0] = true; - batchReply.getDocumentsNotFound()[2] = true; - - std::unique_ptr<mbus::Reply> mbusReply = msg->createReply(); - documentapi::BatchDocumentUpdateReply* mbusBatchReply( - dynamic_cast<documentapi::BatchDocumentUpdateReply*>(mbusReply.get())); - CPPUNIT_ASSERT(mbusBatchReply != 0); - - _converter->transferReplyState(batchReply, *mbusReply); - - CPPUNIT_ASSERT_EQUAL(std::size_t(3), mbusBatchReply->getDocumentsNotFound().size()); - CPPUNIT_ASSERT(mbusBatchReply->getDocumentsNotFound()[0] == true); - CPPUNIT_ASSERT(mbusBatchReply->getDocumentsNotFound()[1] == false); - CPPUNIT_ASSERT(mbusBatchReply->getDocumentsNotFound()[2] == true); -} - -void DocumentApiConverterTest::testStatBucket() { documentapi::StatBucketMessage msg(BucketId(123), ""); diff --git a/storage/src/vespa/storage/common/messagebucket.cpp b/storage/src/vespa/storage/common/messagebucket.cpp index ecbad310a58..4a6f638262d 100644 --- a/storage/src/vespa/storage/common/messagebucket.cpp +++ b/storage/src/vespa/storage/common/messagebucket.cpp @@ -9,7 +9,6 @@ #include <vespa/storageapi/message/removelocation.h> #include <vespa/storage/persistence/messages.h> #include <vespa/storageapi/message/stat.h> -#include <vespa/storageapi/message/batch.h> #include <vespa/vespalib/util/exceptions.h> @@ -31,8 +30,6 @@ getStorageMessageBucket(const api::StorageMessage& msg) return static_cast<const api::RevertCommand&>(msg).getBucket(); case api::MessageType::STATBUCKET_ID: return static_cast<const api::StatBucketCommand&>(msg).getBucket(); - case api::MessageType::BATCHPUTREMOVE_ID: - return static_cast<const api::BatchPutRemoveCommand&>(msg).getBucket(); case api::MessageType::REMOVELOCATION_ID: return static_cast<const api::RemoveLocationCommand&>(msg).getBucket(); case api::MessageType::CREATEBUCKET_ID: diff --git a/storage/src/vespa/storage/distributor/distributor.cpp b/storage/src/vespa/storage/distributor/distributor.cpp index 2d490ea9923..1664dd0d9a1 100644 --- a/storage/src/vespa/storage/distributor/distributor.cpp +++ b/storage/src/vespa/storage/distributor/distributor.cpp @@ -616,8 +616,6 @@ bool is_client_request(const api::StorageMessage& msg) noexcept { case api::MessageType::STATBUCKET_ID: case api::MessageType::UPDATE_ID: case api::MessageType::REMOVELOCATION_ID: - case api::MessageType::BATCHPUTREMOVE_ID: // Deprecated - case api::MessageType::BATCHDOCUMENTUPDATE_ID: // Deprecated return true; default: return false; diff --git a/storage/src/vespa/storage/distributor/externaloperationhandler.cpp b/storage/src/vespa/storage/distributor/externaloperationhandler.cpp index bb8c2b0608a..2d27b91f3e3 100644 --- a/storage/src/vespa/storage/distributor/externaloperationhandler.cpp +++ b/storage/src/vespa/storage/distributor/externaloperationhandler.cpp @@ -15,7 +15,6 @@ #include <vespa/document/util/stringutil.h> #include <vespa/storageapi/message/persistence.h> #include <vespa/storageapi/message/removelocation.h> -#include <vespa/storageapi/message/batch.h> #include <vespa/storageapi/message/stat.h> #include "distributor_bucket_space_repo.h" #include "distributor_bucket_space.h" diff --git a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp index 0b0fc05763f..c652f787b2e 100644 --- a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp @@ -6,8 +6,8 @@ #include "updateoperation.h" #include <vespa/storage/distributor/distributor_bucket_space.h> #include <vespa/storageapi/message/persistence.h> -#include <vespa/storageapi/message/batch.h> #include <vespa/document/datatype/documenttype.h> +#include <vespa/document/fieldvalue/document.h> #include <vespa/document/select/parser.h> #include <vespa/vespalib/stllike/hash_map.hpp> diff --git a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h index e3fb6c93a3a..6efce913e70 100644 --- a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h @@ -15,7 +15,6 @@ namespace storage { namespace api { class UpdateCommand; -class BatchDocumentUpdateCommand; class CreateBucketReply; } diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/mergeoperation.cpp b/storage/src/vespa/storage/distributor/operations/idealstate/mergeoperation.cpp index 3475e3e12ba..784ea7253b6 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/mergeoperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/idealstate/mergeoperation.cpp @@ -308,9 +308,7 @@ constexpr std::array<uint32_t, 7> WRITE_FEED_MESSAGE_TYPES {{ api::MessageType::PUT_ID, api::MessageType::REMOVE_ID, api::MessageType::UPDATE_ID, - api::MessageType::REMOVELOCATION_ID, - api::MessageType::BATCHPUTREMOVE_ID, - api::MessageType::BATCHDOCUMENTUPDATE_ID + api::MessageType::REMOVELOCATION_ID }}; } diff --git a/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp b/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp index f9571228ef9..3ccb35ef5a7 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp +++ b/storage/src/vespa/storage/persistence/filestorage/filestorhandlerimpl.cpp @@ -12,7 +12,6 @@ #include <vespa/storage/common/messagebucket.h> #include <vespa/storage/persistence/messages.h> #include <vespa/storageapi/message/stat.h> -#include <vespa/storageapi/message/batch.h> #include <vespa/vespalib/stllike/hash_map.hpp> #include <vespa/vespalib/util/exceptions.h> @@ -249,8 +248,6 @@ FileStorHandlerImpl::messageMayBeAborted(const api::StorageMessage& msg) case api::MessageType::JOINBUCKETS_ID: case api::MessageType::UPDATE_ID: case api::MessageType::REMOVELOCATION_ID: - case api::MessageType::BATCHPUTREMOVE_ID: - case api::MessageType::BATCHDOCUMENTUPDATE_ID: case api::MessageType::SETBUCKETSTATE_ID: return true; default: @@ -587,7 +584,6 @@ FileStorHandlerImpl::remapMessage(api::StorageMessage& msg, const document::Buck break; } case api::MessageType::STAT_ID: - case api::MessageType::BATCHPUTREMOVE_ID: case api::MessageType::REVERT_ID: case api::MessageType::REMOVELOCATION_ID: case api::MessageType::SETBUCKETSTATE_ID: diff --git a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp index bf0244255c1..e7323d07480 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp +++ b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp @@ -11,7 +11,6 @@ #include <vespa/storage/config/config-stor-server.h> #include <vespa/storage/persistence/bucketownershipnotifier.h> #include <vespa/storage/persistence/persistencethread.h> -#include <vespa/storageapi/message/batch.h> #include <vespa/storageapi/message/bucketsplitting.h> #include <vespa/storageapi/message/state.h> #include <vespa/vespalib/stllike/hash_map.hpp> @@ -325,16 +324,6 @@ FileStorManager::onRevert(const shared_ptr<api::RevertCommand>& cmd) } bool -FileStorManager::onBatchPutRemove(const std::shared_ptr<api::BatchPutRemoveCommand>& cmd) -{ - StorBucketDatabase::WrappedEntry entry(mapOperationToBucketAndDisk(*cmd, 0)); - if (entry.exist()) { - handlePersistenceMessage(cmd, entry->disk); - } - return true; -} - -bool FileStorManager::onRemoveLocation(const std::shared_ptr<api::RemoveLocationCommand>& cmd) { StorBucketDatabase::WrappedEntry entry(mapOperationToDisk(*cmd, cmd->getBucket())); diff --git a/storage/src/vespa/storage/persistence/filestorage/filestormanager.h b/storage/src/vespa/storage/persistence/filestorage/filestormanager.h index 4bf2c1049cf..5c52e6c6a23 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestormanager.h +++ b/storage/src/vespa/storage/persistence/filestorage/filestormanager.h @@ -151,7 +151,6 @@ private: bool onGet(const std::shared_ptr<api::GetCommand>&) override; bool onRemove(const std::shared_ptr<api::RemoveCommand>&) override; bool onRevert(const std::shared_ptr<api::RevertCommand>&) override; - bool onBatchPutRemove(const std::shared_ptr<api::BatchPutRemoveCommand>&) override; bool onStatBucket(const std::shared_ptr<api::StatBucketCommand>&) override; // Bucket operations diff --git a/storage/src/vespa/storage/storageserver/documentapiconverter.cpp b/storage/src/vespa/storage/storageserver/documentapiconverter.cpp index 00bc395b536..b6ac2a4b219 100644 --- a/storage/src/vespa/storage/storageserver/documentapiconverter.cpp +++ b/storage/src/vespa/storage/storageserver/documentapiconverter.cpp @@ -3,9 +3,9 @@ #include "documentapiconverter.h" #include "priorityconverter.h" #include <vespa/document/bucket/bucketidfactory.h> +#include <vespa/document/update/documentupdate.h> #include <vespa/documentapi/documentapi.h> #include <vespa/storage/common/bucket_resolver.h> -#include <vespa/storageapi/message/batch.h> #include <vespa/storageapi/message/datagram.h> #include <vespa/storageapi/message/documentsummary.h> #include <vespa/storageapi/message/persistence.h> @@ -100,12 +100,6 @@ DocumentApiConverter::toStorageAPI(documentapi::DocumentMessage& fromMsg) toMsg = std::make_unique<api::DestroyVisitorCommand>(from.getInstanceId()); break; } - case DocumentProtocol::MESSAGE_BATCHDOCUMENTUPDATE: - { - documentapi::BatchDocumentUpdateMessage& from(static_cast<documentapi::BatchDocumentUpdateMessage&>(fromMsg)); - toMsg = std::make_unique<api::BatchDocumentUpdateCommand>(from.getUpdates()); - break; - } case DocumentProtocol::MESSAGE_STATBUCKET: { documentapi::StatBucketMessage& from(static_cast<documentapi::StatBucketMessage&>(fromMsg)); @@ -377,10 +371,6 @@ DocumentApiConverter::transferReplyState(api::StorageReply& fromMsg, mbus::Reply documentapi::CreateVisitorReply& to(static_cast<documentapi::CreateVisitorReply&>(toMsg)); to.setLastBucket(from.getLastBucket()); to.setVisitorStatistics(from.getVisitorStatistics()); - } else if (toMsg.getType() == DocumentProtocol::REPLY_BATCHDOCUMENTUPDATE) { - api::BatchDocumentUpdateReply& from(static_cast<api::BatchDocumentUpdateReply&>(fromMsg)); - documentapi::BatchDocumentUpdateReply& to(static_cast<documentapi::BatchDocumentUpdateReply&>(toMsg)); - to.getDocumentsNotFound() = from.getDocumentsNotFound(); } } diff --git a/storageapi/src/tests/mbusprot/storageprotocoltest.cpp b/storageapi/src/tests/mbusprot/storageprotocoltest.cpp index cfc69bcde45..da7e8cb743e 100644 --- a/storageapi/src/tests/mbusprot/storageprotocoltest.cpp +++ b/storageapi/src/tests/mbusprot/storageprotocoltest.cpp @@ -5,7 +5,6 @@ #include <vespa/storageapi/message/bucketsplitting.h> #include <vespa/storageapi/message/internal.h> #include <vespa/storageapi/message/removelocation.h> -#include <vespa/storageapi/message/batch.h> #include <vespa/storageapi/mbusprot/storageprotocol.h> #include <vespa/storageapi/mbusprot/storagecommand.h> #include <vespa/storageapi/mbusprot/storagereply.h> @@ -89,7 +88,6 @@ struct StorageProtocolTest : public CppUnit::TestFixture { void testSplitBucket51(); void testSplitBucketChain51(); void testJoinBuckets51(); - void testBatchPutRemove51(); void testCreateVisitor51(); void testDestroyVisitor51(); void testRemoveLocation51(); @@ -133,7 +131,6 @@ struct StorageProtocolTest : public CppUnit::TestFixture { CPPUNIT_TEST(testCreateVisitor51); CPPUNIT_TEST(testDestroyVisitor51); CPPUNIT_TEST(testRemoveLocation51); - CPPUNIT_TEST(testBatchPutRemove51); CPPUNIT_TEST(testInternalMessage); CPPUNIT_TEST(testSetBucketState51); @@ -782,55 +779,6 @@ StorageProtocolTest::testApplyBucketDiff51() recordSerialization50(); } -void -StorageProtocolTest::testBatchPutRemove51() -{ - ScopedName test("testBatchPutRemove51"); - - document::BucketId bucketId(20, 0xf1f1f1f1f1ull); - document::Bucket bucket(makeDocumentBucket(bucketId)); - BatchPutRemoveCommand::SP cmd(new BatchPutRemoveCommand(bucket)); - cmd->addPut(_testDoc, 100); - cmd->addHeaderUpdate(_testDoc, 101, 1234); - cmd->addRemove(_testDoc->getId(), 102); - cmd->forceMsgId(556677); - BatchPutRemoveCommand::SP cmd2(copyCommand(cmd, _version5_1)); - CPPUNIT_ASSERT_EQUAL(bucketId, cmd2->getBucketId()); - CPPUNIT_ASSERT_EQUAL(3, (int)cmd2->getOperationCount()); - CPPUNIT_ASSERT_EQUAL(*_testDoc, *(dynamic_cast<const BatchPutRemoveCommand::PutOperation&>(cmd2->getOperation(0)).document)); - CPPUNIT_ASSERT_EQUAL((uint64_t)100, cmd2->getOperation(0).timestamp); - { - vespalib::nbostream header; - _testDoc->serializeHeader(header); - document::Document headerDoc(_docMan.getTypeRepo(), header); - CPPUNIT_ASSERT_EQUAL( - headerDoc, - *(dynamic_cast<const BatchPutRemoveCommand::HeaderUpdateOperation&>( - cmd2->getOperation(1)).document)); - } - CPPUNIT_ASSERT_EQUAL((uint64_t)101, cmd2->getOperation(1).timestamp); - CPPUNIT_ASSERT_EQUAL(1234, (int)dynamic_cast<const BatchPutRemoveCommand::HeaderUpdateOperation&>(cmd2->getOperation(1)).timestampToUpdate); - CPPUNIT_ASSERT_EQUAL(_testDoc->getId(), dynamic_cast<const BatchPutRemoveCommand::RemoveOperation&>(cmd2->getOperation(2)).documentId); - CPPUNIT_ASSERT_EQUAL((uint64_t)102, cmd2->getOperation(2).timestamp); - CPPUNIT_ASSERT_EQUAL(uint64_t(556677), cmd2->getMsgId()); - - BatchPutRemoveReply::SP reply(new BatchPutRemoveReply(*cmd2)); - reply->getDocumentsNotFound().push_back(document::DocumentId("userdoc:footype:1234:foo1")); - reply->getDocumentsNotFound().push_back(document::DocumentId("userdoc:footype:1234:foo2")); - reply->getDocumentsNotFound().push_back(document::DocumentId("userdoc:footype:1234:foo3")); - - BatchPutRemoveReply::SP reply2(copyReply(reply)); - - CPPUNIT_ASSERT_EQUAL(3, (int)reply2->getDocumentsNotFound().size()); - CPPUNIT_ASSERT_EQUAL(document::DocumentId("userdoc:footype:1234:foo1"), reply2->getDocumentsNotFound()[0]); - CPPUNIT_ASSERT_EQUAL(document::DocumentId("userdoc:footype:1234:foo2"), reply2->getDocumentsNotFound()[1]); - CPPUNIT_ASSERT_EQUAL(document::DocumentId("userdoc:footype:1234:foo3"), reply2->getDocumentsNotFound()[2]); - - recordOutput(*cmd2); - recordOutput(*reply2); - recordSerialization50(); -} - namespace { struct MyCommand : public api::InternalCommand { MyCommand() : InternalCommand(101) {} diff --git a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization.cpp b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization.cpp index d08464da715..172cd6c8de5 100644 --- a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization.cpp +++ b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization.cpp @@ -7,7 +7,6 @@ #include <vespa/storageapi/message/bucketsplitting.h> #include <vespa/storageapi/message/visitor.h> #include <vespa/storageapi/message/removelocation.h> -#include <vespa/storageapi/message/batch.h> #include <vespa/vespalib/util/exceptions.h> @@ -132,12 +131,6 @@ ProtocolSerialization::encode(const api::StorageMessage& msg) const case api::MessageType::REMOVELOCATION_REPLY_ID: onEncode(buf, static_cast<const api::RemoveLocationReply&>(msg)); break; - case api::MessageType::BATCHPUTREMOVE_ID: - onEncode(buf, static_cast<const api::BatchPutRemoveCommand&>(msg)); - break; - case api::MessageType::BATCHPUTREMOVE_REPLY_ID: - onEncode(buf, static_cast<const api::BatchPutRemoveReply&>(msg)); - break; case api::MessageType::SETBUCKETSTATE_ID: onEncode(buf, static_cast<const api::SetBucketStateCommand&>(msg)); break; @@ -205,8 +198,6 @@ ProtocolSerialization::decodeCommand(mbus::BlobRef data) const cmd = onDecodeDestroyVisitorCommand(buf); break; case api::MessageType::REMOVELOCATION_ID: cmd = onDecodeRemoveLocationCommand(buf); break; - case api::MessageType::BATCHPUTREMOVE_ID: - cmd = onDecodeBatchPutRemoveCommand(buf); break; case api::MessageType::SETBUCKETSTATE_ID: cmd = onDecodeSetBucketStateCommand(buf); break; default: @@ -269,8 +260,6 @@ ProtocolSerialization::decodeReply(mbus::BlobRef data, const api::StorageCommand reply = onDecodeDestroyVisitorReply(cmd, buf); break; case api::MessageType::REMOVELOCATION_REPLY_ID: reply = onDecodeRemoveLocationReply(cmd, buf); break; - case api::MessageType::BATCHPUTREMOVE_REPLY_ID: - reply = onDecodeBatchPutRemoveReply(cmd, buf); break; case api::MessageType::SETBUCKETSTATE_REPLY_ID: reply = onDecodeSetBucketStateReply(cmd, buf); break; default: diff --git a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization.h b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization.h index e7d80a0614f..9c3ddb88bdf 100644 --- a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization.h +++ b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization.h @@ -50,10 +50,6 @@ class SetBucketStateReply; class CreateVisitorCommand; class RemoveLocationCommand; class RemoveLocationReply; -class BatchPutRemoveCommand; -class BatchPutRemoveReply; -class BatchDocumentUpdateCommand; -class BatchDocumentUpdateReply; } namespace storage::mbusprot { @@ -125,8 +121,6 @@ protected: virtual void onEncode(GBBuf&, const api::DestroyVisitorReply&) const = 0; virtual void onEncode(GBBuf&, const api::RemoveLocationCommand&) const = 0; virtual void onEncode(GBBuf&, const api::RemoveLocationReply&) const = 0; - virtual void onEncode(GBBuf&, const api::BatchPutRemoveCommand&) const = 0; - virtual void onEncode(GBBuf&, const api::BatchPutRemoveReply&) const = 0; virtual SCmd::UP onDecodePutCommand(BBuf&) const = 0; virtual SRep::UP onDecodePutReply(const SCmd&, BBuf&) const = 0; @@ -166,8 +160,6 @@ protected: virtual SRep::UP onDecodeDestroyVisitorReply(const SCmd&, BBuf&) const = 0; virtual SCmd::UP onDecodeRemoveLocationCommand(BBuf&) const = 0; virtual SRep::UP onDecodeRemoveLocationReply(const SCmd&, BBuf&) const = 0; - virtual SCmd::UP onDecodeBatchPutRemoveCommand(BBuf&) const = 0; - virtual SRep::UP onDecodeBatchPutRemoveReply(const SCmd&, BBuf&) const = 0; virtual document::Bucket getBucket(document::ByteBuffer& buf) const = 0; virtual void putBucket(const document::Bucket& bucket, vespalib::GrowableByteBuffer& buf) const = 0; diff --git a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization4_2.cpp b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization4_2.cpp index 2532f76f3a0..74a0c964d19 100644 --- a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization4_2.cpp +++ b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization4_2.cpp @@ -7,7 +7,6 @@ #include "storagereply.h" #include <vespa/storageapi/message/bucketsplitting.h> -#include <vespa/storageapi/message/batch.h> #include <vespa/storageapi/message/visitor.h> #include <vespa/storageapi/message/removelocation.h> #include <vespa/vespalib/util/exceptions.h> @@ -25,104 +24,6 @@ ProtocolSerialization4_2::ProtocolSerialization4_2( { } -void -ProtocolSerialization4_2::onEncode( - GBBuf& buf, const api::BatchPutRemoveCommand& msg) const -{ - // Serialization format - allow different types of serialization depending on source. - buf.putByte(0); - putBucket(msg.getBucket(), buf); - buf.putInt(msg.getOperationCount()); - - for (uint32_t i = 0; i < msg.getOperationCount(); i++) { - const api::BatchPutRemoveCommand::Operation& op = msg.getOperation(i); - buf.putByte((uint8_t)op.type); - buf.putLong(op.timestamp); - - switch (op.type) { - case api::BatchPutRemoveCommand::Operation::REMOVE: - buf.putString(static_cast<const api::BatchPutRemoveCommand::RemoveOperation&>(op).documentId.toString()); - break; - case api::BatchPutRemoveCommand::Operation::HEADERUPDATE: - { - buf.putLong(static_cast<const api::BatchPutRemoveCommand::HeaderUpdateOperation&>(op).timestampToUpdate); - - vespalib::nbostream stream; - static_cast<const api::BatchPutRemoveCommand::HeaderUpdateOperation&>(op).document->serializeHeader(stream); - buf.putInt(stream.size()); - buf.putBytes(stream.peek(), stream.size()); - break; - } - case api::BatchPutRemoveCommand::Operation::PUT: - SH::putDocument(static_cast<const api::BatchPutRemoveCommand::PutOperation&>(op).document.get(), buf); - break; - } - } - onEncodeBucketInfoCommand(buf, msg); -} - -api::StorageCommand::UP -ProtocolSerialization4_2::onDecodeBatchPutRemoveCommand(BBuf& buf) const -{ - SH::getByte(buf); - document::Bucket bucket = getBucket(buf); - std::unique_ptr<api::BatchPutRemoveCommand> cmd(new api::BatchPutRemoveCommand(bucket)); - int length = SH::getInt(buf); - - for (int i = 0; i < length; i++) { - int type = SH::getByte(buf); - long timestamp = SH::getLong(buf); - - switch (type) { - case api::BatchPutRemoveCommand::Operation::REMOVE: - cmd->addRemove(document::DocumentId(SH::getString(buf)), timestamp); - break; - case api::BatchPutRemoveCommand::Operation::HEADERUPDATE: - { - long newTimestamp = SH::getLong(buf); - cmd->addHeaderUpdate(document::Document::SP( - SH::getDocument(buf, getTypeRepo())), - timestamp, newTimestamp); - break; - } - case api::BatchPutRemoveCommand::Operation::PUT: - cmd->addPut(document::Document::SP(SH::getDocument( - buf, getTypeRepo())), timestamp); - break; - } - } - - onDecodeBucketInfoCommand(buf, *cmd); - - return api::StorageCommand::UP(cmd.release()); -} - -void ProtocolSerialization4_2::onEncode( - GBBuf& buf, const api::BatchPutRemoveReply& msg) const -{ - buf.putInt(msg.getDocumentsNotFound().size()); - for (uint32_t i = 0; i < msg.getDocumentsNotFound().size(); i++) { - buf.putString(msg.getDocumentsNotFound()[i].toString()); - } - - onEncodeBucketInfoReply(buf, msg); -} - -api::StorageReply::UP -ProtocolSerialization4_2::onDecodeBatchPutRemoveReply(const SCmd& cmd, - BBuf& buf) const -{ - api::BatchPutRemoveReply::UP msg(new api::BatchPutRemoveReply( - static_cast<const api::BatchPutRemoveCommand&>(cmd))); - uint32_t count = SH::getInt(buf); - for (uint32_t i = 0; i < count; i++) { - msg->getDocumentsNotFound().push_back(document::DocumentId(SH::getString(buf))); - } - - onDecodeBucketInfoReply(buf, *msg); - return api::StorageReply::UP(msg.release()); -} - void ProtocolSerialization4_2::onEncode( GBBuf& buf, const api::GetCommand& msg) const { diff --git a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization4_2.h b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization4_2.h index 3a6f77e46a3..56aa3d4ed30 100644 --- a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization4_2.h +++ b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization4_2.h @@ -28,8 +28,6 @@ protected: void onEncode(GBBuf&, const api::RemoveLocationReply&) const override; // Not supported on 4.2, but implemented here for simplicity. - void onEncode(GBBuf&, const api::BatchPutRemoveCommand&) const override; - void onEncode(GBBuf&, const api::BatchPutRemoveReply&) const override; void onEncode(GBBuf&, const api::SetBucketStateCommand&) const override; void onEncode(GBBuf&, const api::SetBucketStateReply&) const override; @@ -59,10 +57,6 @@ protected: SCmd::UP onDecodeRemoveLocationCommand(BBuf&) const override; SRep::UP onDecodeRemoveLocationReply(const SCmd&, BBuf&) const override; - // Not supported on 4.2, but implemented here for simplicity. - SCmd::UP onDecodeBatchPutRemoveCommand(BBuf&) const override; - SRep::UP onDecodeBatchPutRemoveReply(const SCmd&, BBuf&) const override; - virtual void onDecodeBucketInfoCommand(BBuf&, api::BucketInfoCommand&) const; virtual void onDecodeBucketInfoReply(BBuf&, api::BucketInfoReply&) const = 0; virtual void onDecodeCommand(BBuf& buf, api::StorageCommand& msg) const = 0; diff --git a/storageapi/src/vespa/storageapi/message/CMakeLists.txt b/storageapi/src/vespa/storageapi/message/CMakeLists.txt index dbbaad8eed1..cde9183482f 100644 --- a/storageapi/src/vespa/storageapi/message/CMakeLists.txt +++ b/storageapi/src/vespa/storageapi/message/CMakeLists.txt @@ -12,7 +12,6 @@ vespa_add_library(storageapi_message OBJECT stat.cpp removelocation.cpp queryresult.cpp - batch.cpp internal.cpp DEPENDS ) diff --git a/storageapi/src/vespa/storageapi/message/batch.cpp b/storageapi/src/vespa/storageapi/message/batch.cpp deleted file mode 100644 index de8ac849dee..00000000000 --- a/storageapi/src/vespa/storageapi/message/batch.cpp +++ /dev/null @@ -1,173 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -// -#include "batch.h" -#include <vespa/document/bucket/bucketidfactory.h> -#include <ostream> - -using namespace storage::api; -using document::BucketSpace; - -IMPLEMENT_COMMAND(BatchPutRemoveCommand, BatchPutRemoveReply) -IMPLEMENT_REPLY(BatchPutRemoveReply) -IMPLEMENT_COMMAND(BatchDocumentUpdateCommand, BatchDocumentUpdateReply) -IMPLEMENT_REPLY(BatchDocumentUpdateReply) - - -BatchPutRemoveCommand::Operation::Operation(uint64_t ts, Type tp) - : timestamp(ts), - type(tp) -{ -} - -BatchPutRemoveCommand::PutOperation::PutOperation(document::Document::SP doc, uint64_t ts) - : Operation(ts, PUT), - document(doc) -{ -} - -BatchPutRemoveCommand::HeaderUpdateOperation::HeaderUpdateOperation(document::Document::SP doc, uint64_t newTimestamp, uint64_t timestampToUpdate_) - : Operation(newTimestamp, HEADERUPDATE), - document(doc), - timestampToUpdate(timestampToUpdate_) -{ -} - -BatchPutRemoveCommand::RemoveOperation::RemoveOperation(const document::DocumentId& docId, uint64_t ts) - : Operation(ts, REMOVE), - documentId(docId) -{ -} - -BatchPutRemoveCommand::BatchPutRemoveCommand(const document::Bucket &bucket) - : BucketInfoCommand(MessageType::BATCHPUTREMOVE, bucket), - _approxSize(0) -{ -} - -void -BatchPutRemoveCommand::addPut(document::Document::SP document, uint64_t ts) -{ - _operations.push_back(std::unique_ptr<Operation>(new PutOperation(document, ts))); - _approxSize += document->serialize()->getLength(); -} - -void -BatchPutRemoveCommand::addHeaderUpdate(document::Document::SP document, uint64_t ts, uint64_t timestampToUpdate) -{ - _operations.push_back(std::unique_ptr<Operation>(new HeaderUpdateOperation(document, ts, timestampToUpdate))); - _approxSize += document->serialize()->getLength(); -} - -void -BatchPutRemoveCommand::addRemove(const document::DocumentId& docId, uint64_t ts) -{ - _operations.push_back(std::unique_ptr<Operation>(new RemoveOperation(docId, ts))); - _approxSize += docId.toString().length(); -} - -void -BatchPutRemoveCommand::addOperation(const Operation& op, bool cloneDocument) -{ - switch (op.type) { - case Operation::PUT: - { - document::Document::SP doc; - if (!cloneDocument) { - doc = static_cast<const PutOperation&>(op).document; - } else { - doc.reset(static_cast<const PutOperation&>(op).document->clone()); - } - addPut(doc, op.timestamp); - break; - } - case Operation::REMOVE: - addRemove(static_cast<const RemoveOperation&>(op).documentId, op.timestamp); - break; - case Operation::HEADERUPDATE: - { - const HeaderUpdateOperation& hup = static_cast<const HeaderUpdateOperation&>(op); - document::Document::SP doc; - if (!cloneDocument) { - doc = hup.document; - } else { - doc.reset(hup.document->clone()); - } - addHeaderUpdate(doc, op.timestamp, hup.timestampToUpdate); - break; - } - } -} - -void -BatchPutRemoveCommand::print(std::ostream& out, bool verbose, - const std::string& indent) const { - out << "BatchPutRemove(" << getBucketId() << ", " << _operations.size() << " operations)"; - - if (verbose) { - out << " : "; - BucketInfoCommand::print(out, verbose, indent); - } -} - -BatchPutRemoveReply::BatchPutRemoveReply(const BatchPutRemoveCommand& cmd) - : BucketInfoReply(cmd) -{ -} - -void -BatchPutRemoveReply::print(std::ostream& out, bool verbose, - const std::string& indent) const { - out << "BatchPutRemoveReply("; - out << _documentsNotFound.size() << " documents not found)"; - - if (verbose) { - out << " {"; - for (std::vector<document::DocumentId>::const_iterator it = - _documentsNotFound.begin(); - it != _documentsNotFound.end(); ++it) - { - out << "\n" << indent << " " << (*it); - } - out << "\n" << indent << "} : "; - BucketInfoReply::print(out, verbose, indent); - } -} - -BatchDocumentUpdateCommand::BatchDocumentUpdateCommand(const UpdateList& updates) - : StorageCommand(MessageType::BATCHDOCUMENTUPDATE), - _updates(updates), - _bucket(BucketSpace::placeHolder(), document::BucketId()) -{ - document::BucketIdFactory factory; - _bucket = document::Bucket(BucketSpace::placeHolder(), factory.getBucketId(updates[0]->getId())); -} - -void -BatchDocumentUpdateCommand::print(std::ostream& out, bool verbose, - const std::string& indent) const { - out << "BatchDocumentUpdate(" << _updates.size() << " operations)"; - - if (verbose) { - out << " : "; - StorageCommand::print(out, verbose, indent); - } -} - -BatchDocumentUpdateReply::BatchDocumentUpdateReply(const BatchDocumentUpdateCommand& cmd) - : StorageReply(cmd), - _documentsNotFound() -{ -} - -void -BatchDocumentUpdateReply::print(std::ostream& out, bool verbose, - const std::string& indent) const { - out << "BatchDocumentUpdateReply(" - << std::count(_documentsNotFound.begin(), _documentsNotFound.end(), true) - << " not found)"; - - if (verbose) { - out << " : "; - StorageReply::print(out, verbose, indent); - } -} diff --git a/storageapi/src/vespa/storageapi/message/batch.h b/storageapi/src/vespa/storageapi/message/batch.h deleted file mode 100644 index 56cca3cfe38..00000000000 --- a/storageapi/src/vespa/storageapi/message/batch.h +++ /dev/null @@ -1,186 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#pragma once - -#include <vespa/document/fieldvalue/document.h> -#include <vespa/document/update/documentupdate.h> -#include <vespa/storageapi/messageapi/bucketinfocommand.h> -#include <vespa/storageapi/messageapi/bucketinforeply.h> - -namespace storage::api { - -/** - * @class BatchPutRemoveCommand - * @ingroup message - * - * @brief Sends a batch of puts and removes - */ -class BatchPutRemoveCommand : public BucketInfoCommand { -public: - class Operation { - public: - enum Type { - REMOVE, // Removes a document - HEADERUPDATE, // Updates the header of a document, if it already exists. - PUT // Inserts a new document. - }; - - Operation(uint64_t ts, Type type); - virtual ~Operation() {}; - - uint64_t timestamp; - Type type; - - virtual const document::DocumentId& getDocumentId() const = 0; - }; - - explicit BatchPutRemoveCommand(const document::Bucket &bucket); - - class PutOperation : public Operation { - public: - PutOperation(document::Document::SP document, uint64_t timestamp); - - document::Document::SP document; - - const document::DocumentId& getDocumentId() const override { - return document->getId(); - } - }; - - class HeaderUpdateOperation : public Operation { - public: - HeaderUpdateOperation(document::Document::SP document, uint64_t newTimestamp, uint64_t timestampToUpdate); - - document::Document::SP document; - uint64_t timestampToUpdate; - - const document::DocumentId& getDocumentId() const override { - return document->getId(); - } - }; - - class RemoveOperation : public Operation { - public: - RemoveOperation(const document::DocumentId& docId, uint64_t timestamp); - - document::DocumentId documentId; - - const document::DocumentId& getDocumentId() const override { - return documentId; - } - }; - - /** - Adds a PUT operation to be performed. - */ - void addPut(document::Document::SP document, uint64_t timestamp); - - /** - Adds a PUT operation to be performed. - */ - void addHeaderUpdate(document::Document::SP document, uint64_t newTimestamp, uint64_t timestampToUpdate); - - /** - Adds a REMOVE operation to be performed. - */ - void addRemove(const document::DocumentId& docId, uint64_t timestamp); - - /** - * Adds an operation to be performed. Optionally deep-clones the - * operation's document. - */ - void addOperation(const Operation& op, bool cloneDocument = false); - - /** - Returns the number of operations in this batch. - */ - uint32_t getOperationCount() const { return _operations.size(); } - - /** - Returns the nth operation in this batch. - */ - const Operation& getOperation(uint32_t index) const { return *_operations[index]; } - - /** - Returns the nth operation in this batch. - */ - Operation& getOperation(uint32_t index) { return *_operations[index]; } - - void print(std::ostream& out, bool verbose, const std::string& indent) const override; - - DECLARE_STORAGECOMMAND(BatchPutRemoveCommand, onBatchPutRemove) - -private: - std::vector<std::unique_ptr<Operation> > _operations; - uint32_t _approxSize; -}; - -/** - * @class BatchPutRemoveReply - * @ingroup message - * - * @brief Confirm that a given docoperations have been received. - */ -class BatchPutRemoveReply : public BucketInfoReply { -private: - std::vector<document::DocumentId> _documentsNotFound; - -public: - explicit BatchPutRemoveReply(const BatchPutRemoveCommand&); - - void print(std::ostream& out, bool verbose, const std::string& indent) const override; - - const std::vector<document::DocumentId>& getDocumentsNotFound() const { return _documentsNotFound; } - std::vector<document::DocumentId>& getDocumentsNotFound() { return _documentsNotFound; } - - DECLARE_STORAGEREPLY(BatchPutRemoveReply, onBatchPutRemoveReply) -}; - -class BatchDocumentUpdateCommand : public StorageCommand -{ -public: - typedef std::vector<document::DocumentUpdate::SP > UpdateList; - - /** - Creates a batch update message containing the given updates. - */ - BatchDocumentUpdateCommand(const UpdateList& updates); - - /** - @return Returns a list of the updates to be performed. - */ - const UpdateList& getUpdates() const { return _updates; }; - void print(std::ostream& out, bool verbose, const std::string& indent) const override; - - /** - Returns a bucket suitable for routing this message. - */ - document::Bucket getBucket() const override { return _bucket; } - bool hasSingleBucketId() const override { return true; } - - DECLARE_STORAGECOMMAND(BatchDocumentUpdateCommand, onBatchDocumentUpdate) - -private: - UpdateList _updates; - document::Bucket _bucket; -}; - -/** - * @class BatchDocumentUpdateReply - * @ingroup message - * - * @brief Confirm that a given docoperations have been received. - */ -class BatchDocumentUpdateReply : public StorageReply { - // 1-1 mapping of found/not found state for documents - std::vector<bool> _documentsNotFound; -public: - explicit BatchDocumentUpdateReply(const BatchDocumentUpdateCommand&); - void print(std::ostream& out, bool verbose, const std::string& indent) const override; - const std::vector<bool>& getDocumentsNotFound() const { return _documentsNotFound; } - std::vector<bool>& getDocumentsNotFound() { return _documentsNotFound; } - - DECLARE_STORAGEREPLY(BatchDocumentUpdateReply, onBatchDocumentUpdateReply) -}; - -} diff --git a/storageapi/src/vespa/storageapi/messageapi/messagehandler.h b/storageapi/src/vespa/storageapi/messageapi/messagehandler.h index e7bd2b5bf27..a9c1dfb8f26 100644 --- a/storageapi/src/vespa/storageapi/messageapi/messagehandler.h +++ b/storageapi/src/vespa/storageapi/messageapi/messagehandler.h @@ -25,8 +25,6 @@ class PutCommand; // Add document class UpdateCommand; // Update document class RemoveCommand; // Remove document class RevertCommand; // Revert put/remove operation -class BatchPutRemoveCommand; -class BatchDocumentUpdateCommand; class CreateVisitorCommand; // Create a new visitor class DestroyVisitorCommand; // Destroy a running visitor @@ -64,8 +62,6 @@ class PutReply; class UpdateReply; class RemoveReply; class RevertReply; -class BatchPutRemoveReply; -class BatchDocumentUpdateReply; class CreateVisitorReply; class DestroyVisitorReply; @@ -143,18 +139,6 @@ public: { return false; } virtual bool onRevertReply(const std::shared_ptr<api::RevertReply>&) { return false; } - virtual bool onBatchPutRemove( - const std::shared_ptr<api::BatchPutRemoveCommand>&) - { return false; } - virtual bool onBatchPutRemoveReply( - const std::shared_ptr<api::BatchPutRemoveReply>&) - { return false; } - virtual bool onBatchDocumentUpdate( - const std::shared_ptr<api::BatchDocumentUpdateCommand>&) - { return false; } - virtual bool onBatchDocumentUpdateReply( - const std::shared_ptr<api::BatchDocumentUpdateReply>&) - { return false; } // Visiting virtual bool onCreateVisitor( diff --git a/storageapi/src/vespa/storageapi/messageapi/storagemessage.cpp b/storageapi/src/vespa/storageapi/messageapi/storagemessage.cpp index 563a2aab7c1..bab475eea32 100644 --- a/storageapi/src/vespa/storageapi/messageapi/storagemessage.cpp +++ b/storageapi/src/vespa/storageapi/messageapi/storagemessage.cpp @@ -105,10 +105,6 @@ const MessageType MessageType::REMOVELOCATION("Removelocation", REMOVELOCATION_I const MessageType MessageType::REMOVELOCATION_REPLY("Removelocation Reply", REMOVELOCATION_REPLY_ID, &MessageType::REMOVELOCATION); const MessageType MessageType::QUERYRESULT("QueryResult", QUERYRESULT_ID); const MessageType MessageType::QUERYRESULT_REPLY("QueryResult reply", QUERYRESULT_REPLY_ID, &MessageType::QUERYRESULT); -const MessageType MessageType::BATCHPUTREMOVE("BatchPutRemove", BATCHPUTREMOVE_ID); -const MessageType MessageType::BATCHPUTREMOVE_REPLY("BatchPutRemove reply", BATCHPUTREMOVE_REPLY_ID, &MessageType::BATCHPUTREMOVE); -const MessageType MessageType::BATCHDOCUMENTUPDATE("BatchDocumentUpdate", BATCHDOCUMENTUPDATE_ID); -const MessageType MessageType::BATCHDOCUMENTUPDATE_REPLY("BatchDocumentUpdate reply", BATCHDOCUMENTUPDATE_REPLY_ID, &MessageType::BATCHDOCUMENTUPDATE); const MessageType MessageType::SETBUCKETSTATE("SetBucketState", SETBUCKETSTATE_ID); const MessageType MessageType::SETBUCKETSTATE_REPLY("SetBucketStateReply", SETBUCKETSTATE_REPLY_ID, &MessageType::SETBUCKETSTATE); diff --git a/storageapi/src/vespa/storageapi/messageapi/storagemessage.h b/storageapi/src/vespa/storageapi/messageapi/storagemessage.h index c81ee51ae22..c9f6e737a47 100644 --- a/storageapi/src/vespa/storageapi/messageapi/storagemessage.h +++ b/storageapi/src/vespa/storageapi/messageapi/storagemessage.h @@ -147,10 +147,6 @@ public: REMOVELOCATION_REPLY_ID = 87, QUERYRESULT_ID = 88, QUERYRESULT_REPLY_ID = 89, - BATCHPUTREMOVE_ID = 90, - BATCHPUTREMOVE_REPLY_ID = 91, - BATCHDOCUMENTUPDATE_ID = 92, - BATCHDOCUMENTUPDATE_REPLY_ID = 93, SETBUCKETSTATE_ID = 94, SETBUCKETSTATE_REPLY_ID = 95, MESSAGETYPE_MAX_ID @@ -233,10 +229,6 @@ public: static const MessageType REMOVELOCATION_REPLY; static const MessageType QUERYRESULT; static const MessageType QUERYRESULT_REPLY; - static const MessageType BATCHPUTREMOVE; - static const MessageType BATCHPUTREMOVE_REPLY; - static const MessageType BATCHDOCUMENTUPDATE; - static const MessageType BATCHDOCUMENTUPDATE_REPLY; static const MessageType SETBUCKETSTATE; static const MessageType SETBUCKETSTATE_REPLY; diff --git a/vespajlib/pom.xml b/vespajlib/pom.xml index 5b9c143a447..741b4b58cc9 100644 --- a/vespajlib/pom.xml +++ b/vespajlib/pom.xml @@ -31,6 +31,10 @@ <groupId>org.apache.commons</groupId> <artifactId>commons-exec</artifactId> </dependency> + <dependency> + <groupId>net.java.dev.jna</groupId> + <artifactId>jna</artifactId> + </dependency> <!-- provided scope --> diff --git a/vespajlib/src/main/java/com/yahoo/io/NativeIO.java b/vespajlib/src/main/java/com/yahoo/io/NativeIO.java new file mode 100644 index 00000000000..bee43a8653e --- /dev/null +++ b/vespajlib/src/main/java/com/yahoo/io/NativeIO.java @@ -0,0 +1,87 @@ +package com.yahoo.io; + +import java.io.File; +import java.io.FileDescriptor; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.lang.reflect.Field; +import java.util.logging.Logger; + +import com.sun.jna.LastErrorException; +import com.sun.jna.Native; +import com.sun.jna.Platform; + +public class NativeIO { + private final Logger logger = Logger.getLogger(getClass().getName()); + private static final int POSIX_FADV_DONTNEED = 4; // See /usr/include/linux/fadvise.h + private static boolean initialized = false; + private static Throwable initError = null; + static { + try { + if (Platform.isLinux()) { + Native.register(Platform.C_LIBRARY_NAME); + initialized = true; + } + } catch (Throwable throwable) { + initError = throwable; + } + } + + private static final Field fieldFD = getField(FileDescriptor.class, "fd"); + + + private static native int posix_fadvise(int fd, long offset, long len, int flag) throws LastErrorException; + + public NativeIO() { + if (!initialized) { + logger.warning("native IO not possible due to " + getError().getMessage()); + } + } + + public boolean valid() { return initialized; } + public Throwable getError() { + if (initError != null) { + return initError; + } else { + return new RuntimeException("Platform is unsúpported. Only supported on linux."); + } + } + + public void dropFileFromCache(FileDescriptor fd) { + if (initialized) { + posix_fadvise(getNativeFD(fd), 0, 0, POSIX_FADV_DONTNEED); + } + } + + public void dropFileFromCache(File file) { + try { + dropFileFromCache(new FileInputStream(file).getFD()); + } catch (FileNotFoundException e) { + throw new RuntimeException(e); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + + private static Field getField(Class<?> clazz, String fieldName) { + Field field; + try { + field = clazz.getDeclaredField(fieldName); + } catch (NoSuchFieldException e) { + throw new RuntimeException(e); + } + field.setAccessible(true); + return field; + } + + private static int getNativeFD(FileDescriptor fd) { + try { + return fieldFD.getInt(fd); + } catch (IllegalAccessException e) { + throw new RuntimeException(e); + } + } + +} diff --git a/vespajlib/src/test/java/com/yahoo/io/NativeIOTestCase.java b/vespajlib/src/test/java/com/yahoo/io/NativeIOTestCase.java new file mode 100644 index 00000000000..ecd38056a19 --- /dev/null +++ b/vespajlib/src/test/java/com/yahoo/io/NativeIOTestCase.java @@ -0,0 +1,34 @@ +package com.yahoo.io; + +import com.sun.jna.Platform; +import org.junit.Test; + +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertFalse; + +public class NativeIOTestCase { + + @Test + public void requireThatDropFileFromCacheDoesNotThrow() throws IOException { + File testFile = new File("testfile"); + FileOutputStream output = new FileOutputStream(testFile); + output.write('t'); + output.flush(); + output.close(); + NativeIO nativeIO = new NativeIO(); + if (Platform.isLinux()) { + assertTrue(nativeIO.valid()); + } else { + assertFalse(nativeIO.valid()); + assertEquals("Platform is unsúpported. Only supported on linux.", nativeIO.getError().getMessage()); + } + nativeIO.dropFileFromCache(output.getFD()); + nativeIO.dropFileFromCache(testFile); + testFile.delete(); + } +} |