From 62b4faaa78640376d8ec2267b5bbd8fbd4006a85 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Tue, 5 Jun 2018 08:51:00 +0200 Subject: [PATCH 01/13] Docs: Clarify constraints on scripted similarities. (#31076) Scripted similarities provide a lot of flexibility but they still need to obey some rules to not confuse Lucene. --- docs/reference/index-modules/similarity.asciidoc | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/docs/reference/index-modules/similarity.asciidoc b/docs/reference/index-modules/similarity.asciidoc index 795131220ec7b..10975308c3819 100644 --- a/docs/reference/index-modules/similarity.asciidoc +++ b/docs/reference/index-modules/similarity.asciidoc @@ -326,7 +326,18 @@ Which yields: // TESTRESPONSE[s/"took": 12/"took" : $body.took/] // TESTRESPONSE[s/OzrdjxNtQGaqs4DmioFw9A/$body.hits.hits.0._node/] -You might have noticed that a significant part of the script depends on +WARNING: While scripted similarities provide a lot of flexibility, there is +a set of rules that they need to satisfy. Failing to do so could make +Elasticsearch silently return wrong top hits or fail with internal errors at +search time: + + - Returned scores must be positive. + - All other variables remaining equal, scores must not decrease when + `doc.freq` increases. + - All other variables remaining equal, scores must not increase when + `doc.length` increases. + +You might have noticed that a significant part of the above script depends on statistics that are the same for every document. It is possible to make the above slightly more efficient by providing an `weight_script` which will compute the document-independent part of the score and will be available @@ -491,7 +502,6 @@ GET /index/_search?explain=true //////////////////// - Type name: `scripted` [float] From 0d89d80b7165d9f9b3c5aadb0e38139eb52ddff8 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Tue, 5 Jun 2018 08:51:20 +0200 Subject: [PATCH 02/13] Decouple MultiValueMode. (#31075) Currently this class takes care of moth selecting the relevant value, and replacing missing values if any. This is fine for sorting, which always needs to do both at the same time, but we also have a number of aggregations and script utils that need to retain information about missing values so this change proposes to decouple selection of the relevant value and replacement of missing values. --- .../support/MultiValuesSource.java | 2 +- .../expression/DateMethodValueSource.java | 2 +- .../expression/DateObjectValueSource.java | 2 +- .../expression/FieldDataValueSource.java | 2 +- .../index/fielddata/FieldData.java | 91 +++++++---- .../DoubleValuesComparatorSource.java | 3 +- .../FloatValuesComparatorSource.java | 3 +- .../LongValuesComparatorSource.java | 3 +- .../functionscore/DecayFunctionBuilder.java | 9 +- .../elasticsearch/search/MultiValueMode.java | 58 ++----- .../metrics/max/MaxAggregator.java | 2 +- .../metrics/min/MinAggregator.java | 2 +- .../search/sort/GeoDistanceSortBuilder.java | 3 +- .../index/fielddata/FieldDataTests.java | 87 ++++++++++ .../ordinals/MultiOrdinalsTests.java | 3 +- .../search/MultiValueModeTests.java | 153 +++++++++--------- 16 files changed, 260 insertions(+), 165 deletions(-) diff --git a/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSource.java b/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSource.java index 0274c1748dde5..86d1836721f10 100644 --- a/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSource.java +++ b/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSource.java @@ -47,7 +47,7 @@ public NumericDoubleValues getField(final int ordinal, LeafReaderContext ctx) th if (ordinal > names.length) { throw new IndexOutOfBoundsException("ValuesSource array index " + ordinal + " out of bounds"); } - return multiValueMode.select(values[ordinal].doubleValues(ctx), Double.NEGATIVE_INFINITY); + return multiValueMode.select(values[ordinal].doubleValues(ctx)); } } diff --git a/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/DateMethodValueSource.java b/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/DateMethodValueSource.java index b0bc7c203b63b..13845a88fe3f0 100644 --- a/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/DateMethodValueSource.java +++ b/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/DateMethodValueSource.java @@ -54,7 +54,7 @@ class DateMethodValueSource extends FieldDataValueSource { public FunctionValues getValues(Map context, LeafReaderContext leaf) throws IOException { AtomicNumericFieldData leafData = (AtomicNumericFieldData) fieldData.load(leaf); final Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("UTC"), Locale.ROOT); - NumericDoubleValues docValues = multiValueMode.select(leafData.getDoubleValues(), 0d); + NumericDoubleValues docValues = multiValueMode.select(leafData.getDoubleValues()); return new DoubleDocValues(this) { @Override public double doubleVal(int docId) throws IOException { diff --git a/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/DateObjectValueSource.java b/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/DateObjectValueSource.java index 86abcbcbefabd..ee59892a7024f 100644 --- a/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/DateObjectValueSource.java +++ b/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/DateObjectValueSource.java @@ -56,7 +56,7 @@ class DateObjectValueSource extends FieldDataValueSource { public FunctionValues getValues(Map context, LeafReaderContext leaf) throws IOException { AtomicNumericFieldData leafData = (AtomicNumericFieldData) fieldData.load(leaf); MutableDateTime joda = new MutableDateTime(0, DateTimeZone.UTC); - NumericDoubleValues docValues = multiValueMode.select(leafData.getDoubleValues(), 0d); + NumericDoubleValues docValues = multiValueMode.select(leafData.getDoubleValues()); return new DoubleDocValues(this) { @Override public double doubleVal(int docId) throws IOException { diff --git a/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/FieldDataValueSource.java b/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/FieldDataValueSource.java index f2aaced37658e..3e49797bbacc8 100644 --- a/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/FieldDataValueSource.java +++ b/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/FieldDataValueSource.java @@ -68,7 +68,7 @@ public int hashCode() { @SuppressWarnings("rawtypes") // ValueSource uses a rawtype public FunctionValues getValues(Map context, LeafReaderContext leaf) throws IOException { AtomicNumericFieldData leafData = (AtomicNumericFieldData) fieldData.load(leaf); - NumericDoubleValues docValues = multiValueMode.select(leafData.getDoubleValues(), 0d); + NumericDoubleValues docValues = multiValueMode.select(leafData.getDoubleValues()); return new DoubleDocValues(this) { @Override public double doubleVal(int doc) throws IOException { diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/FieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/FieldData.java index b5e1608957e4c..68b8f2c85325f 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/FieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/FieldData.java @@ -291,38 +291,6 @@ public static boolean isMultiValued(SortedSetDocValues values) { return DocValues.unwrapSingleton(values) == null; } - /** - * Returns whether the provided values *might* be multi-valued. There is no - * guarantee that this method will return {@code false} in the single-valued case. - */ - public static boolean isMultiValued(SortedNumericDocValues values) { - return DocValues.unwrapSingleton(values) == null; - } - - /** - * Returns whether the provided values *might* be multi-valued. There is no - * guarantee that this method will return {@code false} in the single-valued case. - */ - public static boolean isMultiValued(SortedNumericDoubleValues values) { - return unwrapSingleton(values) == null; - } - - /** - * Returns whether the provided values *might* be multi-valued. There is no - * guarantee that this method will return {@code false} in the single-valued case. - */ - public static boolean isMultiValued(SortedBinaryDocValues values) { - return unwrapSingleton(values) != null; - } - - /** - * Returns whether the provided values *might* be multi-valued. There is no - * guarantee that this method will return {@code false} in the single-valued case. - */ - public static boolean isMultiValued(MultiGeoPointValues values) { - return unwrapSingleton(values) == null; - } - /** * Return a {@link String} representation of the provided values. That is * typically used for scripts or for the `map` execution mode of terms aggs. @@ -555,4 +523,63 @@ public long nextValue() throws IOException { } } + + /** + * Return a {@link NumericDocValues} instance that has a value for every + * document, returns the same value as {@code values} if there is a value + * for the current document and {@code missing} otherwise. + */ + public static NumericDocValues replaceMissing(NumericDocValues values, long missing) { + return new AbstractNumericDocValues() { + + private long value; + + @Override + public int docID() { + return values.docID(); + } + + @Override + public boolean advanceExact(int target) throws IOException { + if (values.advanceExact(target)) { + value = values.longValue(); + } else { + value = missing; + } + return true; + } + + @Override + public long longValue() throws IOException { + return value; + } + }; + } + + /** + * Return a {@link NumericDoubleValues} instance that has a value for every + * document, returns the same value as {@code values} if there is a value + * for the current document and {@code missing} otherwise. + */ + public static NumericDoubleValues replaceMissing(NumericDoubleValues values, double missing) { + return new NumericDoubleValues() { + + private double value; + + @Override + public boolean advanceExact(int target) throws IOException { + if (values.advanceExact(target)) { + value = values.doubleValue(); + } else { + value = missing; + } + return true; + } + + @Override + public double doubleValue() throws IOException { + return value; + } + }; + } } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java index 338d903a390b3..43bc19a12a384 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java @@ -27,6 +27,7 @@ import org.apache.lucene.search.SortField; import org.apache.lucene.util.BitSet; import org.elasticsearch.common.Nullable; +import org.elasticsearch.index.fielddata.FieldData; import org.elasticsearch.index.fielddata.IndexFieldData; import org.elasticsearch.index.fielddata.IndexNumericFieldData; import org.elasticsearch.index.fielddata.NumericDoubleValues; @@ -71,7 +72,7 @@ protected NumericDocValues getNumericDocValues(LeafReaderContext context, String final SortedNumericDoubleValues values = getValues(context); final NumericDoubleValues selectedValues; if (nested == null) { - selectedValues = sortMode.select(values, dMissingValue); + selectedValues = FieldData.replaceMissing(sortMode.select(values), dMissingValue); } else { final BitSet rootDocs = nested.rootDocs(context); final DocIdSetIterator innerDocs = nested.innerDocs(context); diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java index a61a715547a24..b271dd54bd7fd 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java @@ -25,6 +25,7 @@ import org.apache.lucene.search.SortField; import org.apache.lucene.util.BitSet; import org.elasticsearch.common.Nullable; +import org.elasticsearch.index.fielddata.FieldData; import org.elasticsearch.index.fielddata.IndexFieldData; import org.elasticsearch.index.fielddata.IndexNumericFieldData; import org.elasticsearch.index.fielddata.NumericDoubleValues; @@ -63,7 +64,7 @@ protected NumericDocValues getNumericDocValues(LeafReaderContext context, String final SortedNumericDoubleValues values = indexFieldData.load(context).getDoubleValues(); final NumericDoubleValues selectedValues; if (nested == null) { - selectedValues = sortMode.select(values, dMissingValue); + selectedValues = FieldData.replaceMissing(sortMode.select(values), dMissingValue); } else { final BitSet rootDocs = nested.rootDocs(context); final DocIdSetIterator innerDocs = nested.innerDocs(context); diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java index aa206fe1baebe..362dde6099680 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java @@ -26,6 +26,7 @@ import org.apache.lucene.search.SortField; import org.apache.lucene.util.BitSet; import org.elasticsearch.common.Nullable; +import org.elasticsearch.index.fielddata.FieldData; import org.elasticsearch.index.fielddata.IndexFieldData; import org.elasticsearch.index.fielddata.IndexNumericFieldData; import org.elasticsearch.search.MultiValueMode; @@ -62,7 +63,7 @@ protected NumericDocValues getNumericDocValues(LeafReaderContext context, String final SortedNumericDocValues values = indexFieldData.load(context).getLongValues(); final NumericDocValues selectedValues; if (nested == null) { - selectedValues = sortMode.select(values, dMissingValue); + selectedValues = FieldData.replaceMissing(sortMode.select(values), dMissingValue); } else { final BitSet rootDocs = nested.rootDocs(context); final DocIdSetIterator innerDocs = nested.innerDocs(context); diff --git a/server/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionBuilder.java b/server/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionBuilder.java index 3712040b8de03..54c25b40501d2 100644 --- a/server/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionBuilder.java @@ -40,6 +40,7 @@ import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.index.fielddata.FieldData; import org.elasticsearch.index.fielddata.IndexGeoPointFieldData; import org.elasticsearch.index.fielddata.IndexNumericFieldData; import org.elasticsearch.index.fielddata.MultiGeoPointValues; @@ -354,7 +355,7 @@ public boolean needsScores() { @Override protected NumericDoubleValues distance(LeafReaderContext context) { final MultiGeoPointValues geoPointValues = fieldData.load(context).getGeoPointValues(); - return mode.select(new SortingNumericDoubleValues() { + return FieldData.replaceMissing(mode.select(new SortingNumericDoubleValues() { @Override public boolean advanceExact(int docId) throws IOException { if (geoPointValues.advanceExact(docId)) { @@ -372,7 +373,7 @@ public boolean advanceExact(int docId) throws IOException { return false; } } - }, 0.0); + }), 0); } @Override @@ -436,7 +437,7 @@ public boolean needsScores() { @Override protected NumericDoubleValues distance(LeafReaderContext context) { final SortedNumericDoubleValues doubleValues = fieldData.load(context).getDoubleValues(); - return mode.select(new SortingNumericDoubleValues() { + return FieldData.replaceMissing(mode.select(new SortingNumericDoubleValues() { @Override public boolean advanceExact(int docId) throws IOException { if (doubleValues.advanceExact(docId)) { @@ -451,7 +452,7 @@ public boolean advanceExact(int docId) throws IOException { return false; } } - }, 0.0); + }), 0); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/MultiValueMode.java b/server/src/main/java/org/elasticsearch/search/MultiValueMode.java index b2ee4b8ffbd5f..eaaa5f74fa4d5 100644 --- a/server/src/main/java/org/elasticsearch/search/MultiValueMode.java +++ b/server/src/main/java/org/elasticsearch/search/MultiValueMode.java @@ -411,29 +411,10 @@ public static MultiValueMode fromString(String sortMode) { * * Allowed Modes: SUM, AVG, MEDIAN, MIN, MAX */ - public NumericDocValues select(final SortedNumericDocValues values, final long missingValue) { + public NumericDocValues select(final SortedNumericDocValues values) { final NumericDocValues singleton = DocValues.unwrapSingleton(values); if (singleton != null) { - return new AbstractNumericDocValues() { - - private long value; - - @Override - public boolean advanceExact(int target) throws IOException { - this.value = singleton.advanceExact(target) ? singleton.longValue() : missingValue; - return true; - } - - @Override - public int docID() { - return singleton.docID(); - } - - @Override - public long longValue() throws IOException { - return this.value; - } - }; + return singleton; } else { return new AbstractNumericDocValues() { @@ -441,8 +422,11 @@ public long longValue() throws IOException { @Override public boolean advanceExact(int target) throws IOException { - this.value = values.advanceExact(target) ? pick(values) : missingValue; - return true; + if (values.advanceExact(target)) { + value = pick(values); + return true; + } + return false; } @Override @@ -476,7 +460,7 @@ protected long pick(SortedNumericDocValues values) throws IOException { */ public NumericDocValues select(final SortedNumericDocValues values, final long missingValue, final BitSet parentDocs, final DocIdSetIterator childDocs, int maxDoc) throws IOException { if (parentDocs == null || childDocs == null) { - return select(DocValues.emptySortedNumeric(maxDoc), missingValue); + return FieldData.replaceMissing(DocValues.emptyNumeric(), missingValue); } return new AbstractNumericDocValues() { @@ -529,23 +513,10 @@ protected long pick(SortedNumericDocValues values, long missingValue, DocIdSetIt * * Allowed Modes: SUM, AVG, MEDIAN, MIN, MAX */ - public NumericDoubleValues select(final SortedNumericDoubleValues values, final double missingValue) { + public NumericDoubleValues select(final SortedNumericDoubleValues values) { final NumericDoubleValues singleton = FieldData.unwrapSingleton(values); if (singleton != null) { - return new NumericDoubleValues() { - private double value; - - @Override - public boolean advanceExact(int doc) throws IOException { - this.value = singleton.advanceExact(doc) ? singleton.doubleValue() : missingValue; - return true; - } - - @Override - public double doubleValue() throws IOException { - return this.value; - } - }; + return singleton; } else { return new NumericDoubleValues() { @@ -553,8 +524,11 @@ public double doubleValue() throws IOException { @Override public boolean advanceExact(int target) throws IOException { - value = values.advanceExact(target) ? pick(values) : missingValue; - return true; + if (values.advanceExact(target)) { + value = pick(values); + return true; + } + return false; } @Override @@ -583,7 +557,7 @@ protected double pick(SortedNumericDoubleValues values) throws IOException { */ public NumericDoubleValues select(final SortedNumericDoubleValues values, final double missingValue, final BitSet parentDocs, final DocIdSetIterator childDocs, int maxDoc) throws IOException { if (parentDocs == null || childDocs == null) { - return select(FieldData.emptySortedNumericDoubles(), missingValue); + return FieldData.replaceMissing(FieldData.emptyNumericDouble(), missingValue); } return new NumericDoubleValues() { diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/max/MaxAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/max/MaxAggregator.java index 8ef4d0b7e29d1..ff76e6637baf4 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/max/MaxAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/max/MaxAggregator.java @@ -72,7 +72,7 @@ public LeafBucketCollector getLeafCollector(LeafReaderContext ctx, } final BigArrays bigArrays = context.bigArrays(); final SortedNumericDoubleValues allValues = valuesSource.doubleValues(ctx); - final NumericDoubleValues values = MultiValueMode.MAX.select(allValues, Double.NEGATIVE_INFINITY); + final NumericDoubleValues values = MultiValueMode.MAX.select(allValues); return new LeafBucketCollectorBase(sub, allValues) { @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/min/MinAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/min/MinAggregator.java index f355f55139c04..e4b371514bdf9 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/min/MinAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/min/MinAggregator.java @@ -71,7 +71,7 @@ public LeafBucketCollector getLeafCollector(LeafReaderContext ctx, } final BigArrays bigArrays = context.bigArrays(); final SortedNumericDoubleValues allValues = valuesSource.doubleValues(ctx); - final NumericDoubleValues values = MultiValueMode.MIN.select(allValues, Double.POSITIVE_INFINITY); + final NumericDoubleValues values = MultiValueMode.MIN.select(allValues); return new LeafBucketCollectorBase(sub, allValues) { @Override diff --git a/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java index 1d488a58857df..cfa5a240dea53 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java @@ -41,6 +41,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentParser.Token; +import org.elasticsearch.index.fielddata.FieldData; import org.elasticsearch.index.fielddata.IndexFieldData; import org.elasticsearch.index.fielddata.IndexFieldData.XFieldComparatorSource.Nested; import org.elasticsearch.index.fielddata.IndexGeoPointFieldData; @@ -637,7 +638,7 @@ protected NumericDocValues getNumericDocValues(LeafReaderContext context, String localPoints); final NumericDoubleValues selectedValues; if (nested == null) { - selectedValues = finalSortMode.select(distanceValues, Double.POSITIVE_INFINITY); + selectedValues = FieldData.replaceMissing(finalSortMode.select(distanceValues), Double.POSITIVE_INFINITY); } else { final BitSet rootDocs = nested.rootDocs(context); final DocIdSetIterator innerDocs = nested.innerDocs(context); diff --git a/server/src/test/java/org/elasticsearch/index/fielddata/FieldDataTests.java b/server/src/test/java/org/elasticsearch/index/fielddata/FieldDataTests.java index 6236517dde0be..ac924aa83e45a 100644 --- a/server/src/test/java/org/elasticsearch/index/fielddata/FieldDataTests.java +++ b/server/src/test/java/org/elasticsearch/index/fielddata/FieldDataTests.java @@ -137,4 +137,91 @@ public int docValueCount() { assertEquals(valueBits, asMultiLongs.nextValue()); assertSame(multiValues, FieldData.sortableLongBitsToDoubles(asMultiLongs)); } + + private static NumericDocValues asNumericDocValues(Long... values) { + return new AbstractNumericDocValues() { + + int docID = -1; + + @Override + public int docID() { + return docID; + } + + @Override + public boolean advanceExact(int target) throws IOException { + docID = target; + return target < values.length && values[target] != null; + } + + @Override + public long longValue() throws IOException { + return values[docID]; + } + }; + } + + public void testReplaceMissingLongs() throws IOException { + final NumericDocValues values = asNumericDocValues(null, 3L, 2L, null, 5L, null); + final NumericDocValues replaced = FieldData.replaceMissing(values, 4); + + assertTrue(replaced.advanceExact(0)); + assertEquals(4L, replaced.longValue()); + + assertTrue(replaced.advanceExact(1)); + assertEquals(3L, replaced.longValue()); + + assertTrue(replaced.advanceExact(2)); + assertEquals(2L, replaced.longValue()); + + assertTrue(replaced.advanceExact(3)); + assertEquals(4L, replaced.longValue()); + + assertTrue(replaced.advanceExact(4)); + assertEquals(5L, replaced.longValue()); + + assertTrue(replaced.advanceExact(5)); + assertEquals(4L, replaced.longValue()); + } + + private static NumericDoubleValues asNumericDoubleValues(Double... values) { + return new NumericDoubleValues() { + + int docID = -1; + + @Override + public boolean advanceExact(int target) throws IOException { + docID = target; + return target < values.length && values[target] != null; + } + + @Override + public double doubleValue() throws IOException { + return values[docID]; + } + }; + } + + public void testReplaceMissingDoubles() throws IOException { + final NumericDoubleValues values = asNumericDoubleValues(null, 1.3, 1.2, null, 1.5, null); + final NumericDoubleValues replaced = FieldData.replaceMissing(values, 1.4); + + assertTrue(replaced.advanceExact(0)); + assertEquals(1.4, replaced.doubleValue(), 0d); + + assertTrue(replaced.advanceExact(1)); + assertEquals(1.3, replaced.doubleValue(), 0d); + + assertTrue(replaced.advanceExact(2)); + assertEquals(1.2, replaced.doubleValue(), 0d); + + assertTrue(replaced.advanceExact(3)); + assertEquals(1.4, replaced.doubleValue(), 0d); + + assertTrue(replaced.advanceExact(4)); + assertEquals(1.5, replaced.doubleValue(), 0d); + + assertTrue(replaced.advanceExact(5)); + assertEquals(1.4, replaced.doubleValue(), 0d); + } } diff --git a/server/src/test/java/org/elasticsearch/index/fielddata/ordinals/MultiOrdinalsTests.java b/server/src/test/java/org/elasticsearch/index/fielddata/ordinals/MultiOrdinalsTests.java index 1ae6197c547da..3656d59e788e4 100644 --- a/server/src/test/java/org/elasticsearch/index/fielddata/ordinals/MultiOrdinalsTests.java +++ b/server/src/test/java/org/elasticsearch/index/fielddata/ordinals/MultiOrdinalsTests.java @@ -18,6 +18,7 @@ */ package org.elasticsearch.index.fielddata.ordinals; +import org.apache.lucene.index.DocValues; import org.apache.lucene.index.SortedDocValues; import org.apache.lucene.index.SortedSetDocValues; import org.apache.lucene.util.packed.PackedInts; @@ -261,7 +262,7 @@ private void assertEquals(SortedSetDocValues docs, long[][] ordinalPlan) throws } } assertThat(docs.getValueCount(), equalTo(maxOrd)); - assertThat(FieldData.isMultiValued(docs), equalTo(true)); + assertNull(DocValues.unwrapSingleton(docs)); for (int doc = 0; doc < ordinalPlan.length; ++doc) { long[] ords = ordinalPlan[doc]; assertEquals(ords.length > 0, docs.advanceExact(doc)); diff --git a/server/src/test/java/org/elasticsearch/search/MultiValueModeTests.java b/server/src/test/java/org/elasticsearch/search/MultiValueModeTests.java index d9eb45013263d..b64f6ee0ee3d1 100644 --- a/server/src/test/java/org/elasticsearch/search/MultiValueModeTests.java +++ b/server/src/test/java/org/elasticsearch/search/MultiValueModeTests.java @@ -151,54 +151,55 @@ public int docValueCount() { } private void verifySortedNumeric(Supplier supplier, int maxDoc) throws IOException { - for (long missingValue : new long[] { 0, randomLong() }) { - for (MultiValueMode mode : MultiValueMode.values()) { - SortedNumericDocValues values = supplier.get(); - final NumericDocValues selected = mode.select(values, missingValue); - for (int i = 0; i < maxDoc; ++i) { - assertTrue(selected.advanceExact(i)); - final long actual = selected.longValue(); + for (MultiValueMode mode : MultiValueMode.values()) { + SortedNumericDocValues values = supplier.get(); + final NumericDocValues selected = mode.select(values); + for (int i = 0; i < maxDoc; ++i) { + Long actual = null; + if (selected.advanceExact(i)) { + actual = selected.longValue(); verifyLongValueCanCalledMoreThanOnce(selected, actual); + } - long expected = 0; - if (values.advanceExact(i) == false) { - expected = missingValue; + + Long expected = null; + if (values.advanceExact(i)) { + int numValues = values.docValueCount(); + if (mode == MultiValueMode.MAX) { + expected = Long.MIN_VALUE; + } else if (mode == MultiValueMode.MIN) { + expected = Long.MAX_VALUE; } else { - int numValues = values.docValueCount(); - if (mode == MultiValueMode.MAX) { - expected = Long.MIN_VALUE; + expected = 0L; + } + for (int j = 0; j < numValues; ++j) { + if (mode == MultiValueMode.SUM || mode == MultiValueMode.AVG) { + expected += values.nextValue(); } else if (mode == MultiValueMode.MIN) { - expected = Long.MAX_VALUE; + expected = Math.min(expected, values.nextValue()); + } else if (mode == MultiValueMode.MAX) { + expected = Math.max(expected, values.nextValue()); } - for (int j = 0; j < numValues; ++j) { - if (mode == MultiValueMode.SUM || mode == MultiValueMode.AVG) { - expected += values.nextValue(); - } else if (mode == MultiValueMode.MIN) { - expected = Math.min(expected, values.nextValue()); - } else if (mode == MultiValueMode.MAX) { - expected = Math.max(expected, values.nextValue()); + } + if (mode == MultiValueMode.AVG) { + expected = numValues > 1 ? Math.round((double)expected/(double)numValues) : expected; + } else if (mode == MultiValueMode.MEDIAN) { + int value = numValues/2; + if (numValues % 2 == 0) { + for (int j = 0; j < value - 1; ++j) { + values.nextValue(); } - } - if (mode == MultiValueMode.AVG) { - expected = numValues > 1 ? Math.round((double)expected/(double)numValues) : expected; - } else if (mode == MultiValueMode.MEDIAN) { - int value = numValues/2; - if (numValues % 2 == 0) { - for (int j = 0; j < value - 1; ++j) { - values.nextValue(); - } - expected = Math.round(((double) values.nextValue() + values.nextValue())/2.0); - } else { - for (int j = 0; j < value; ++j) { - values.nextValue(); - } - expected = values.nextValue(); + expected = Math.round(((double) values.nextValue() + values.nextValue())/2.0); + } else { + for (int j = 0; j < value; ++j) { + values.nextValue(); } + expected = values.nextValue(); } } - - assertEquals(mode.toString() + " docId=" + i, expected, actual); } + + assertEquals(mode.toString() + " docId=" + i, expected, actual); } } } @@ -326,54 +327,54 @@ public int docValueCount() { } private void verifySortedNumericDouble(Supplier supplier, int maxDoc) throws IOException { - for (long missingValue : new long[] { 0, randomLong() }) { - for (MultiValueMode mode : MultiValueMode.values()) { - SortedNumericDoubleValues values = supplier.get(); - final NumericDoubleValues selected = mode.select(values, missingValue); - for (int i = 0; i < maxDoc; ++i) { - assertTrue(selected.advanceExact(i)); - final double actual = selected.doubleValue(); + for (MultiValueMode mode : MultiValueMode.values()) { + SortedNumericDoubleValues values = supplier.get(); + final NumericDoubleValues selected = mode.select(values); + for (int i = 0; i < maxDoc; ++i) { + Double actual = null; + if (selected.advanceExact(i)) { + actual = selected.doubleValue(); verifyDoubleValueCanCalledMoreThanOnce(selected, actual); + } - double expected = 0.0; - if (values.advanceExact(i) == false) { - expected = missingValue; + Double expected = null; + if (values.advanceExact(i)) { + int numValues = values.docValueCount(); + if (mode == MultiValueMode.MAX) { + expected = Double.NEGATIVE_INFINITY; + } else if (mode == MultiValueMode.MIN) { + expected = Double.POSITIVE_INFINITY; } else { - int numValues = values.docValueCount(); - if (mode == MultiValueMode.MAX) { - expected = Long.MIN_VALUE; + expected = 0d; + } + for (int j = 0; j < numValues; ++j) { + if (mode == MultiValueMode.SUM || mode == MultiValueMode.AVG) { + expected += values.nextValue(); } else if (mode == MultiValueMode.MIN) { - expected = Long.MAX_VALUE; + expected = Math.min(expected, values.nextValue()); + } else if (mode == MultiValueMode.MAX) { + expected = Math.max(expected, values.nextValue()); } - for (int j = 0; j < numValues; ++j) { - if (mode == MultiValueMode.SUM || mode == MultiValueMode.AVG) { - expected += values.nextValue(); - } else if (mode == MultiValueMode.MIN) { - expected = Math.min(expected, values.nextValue()); - } else if (mode == MultiValueMode.MAX) { - expected = Math.max(expected, values.nextValue()); + } + if (mode == MultiValueMode.AVG) { + expected = expected/numValues; + } else if (mode == MultiValueMode.MEDIAN) { + int value = numValues/2; + if (numValues % 2 == 0) { + for (int j = 0; j < value - 1; ++j) { + values.nextValue(); } - } - if (mode == MultiValueMode.AVG) { - expected = expected/numValues; - } else if (mode == MultiValueMode.MEDIAN) { - int value = numValues/2; - if (numValues % 2 == 0) { - for (int j = 0; j < value - 1; ++j) { - values.nextValue(); - } - expected = (values.nextValue() + values.nextValue())/2.0; - } else { - for (int j = 0; j < value; ++j) { - values.nextValue(); - } - expected = values.nextValue(); + expected = (values.nextValue() + values.nextValue())/2.0; + } else { + for (int j = 0; j < value; ++j) { + values.nextValue(); } + expected = values.nextValue(); } } - - assertEquals(mode.toString() + " docId=" + i, expected, actual, 0.1); } + + assertEquals(mode.toString() + " docId=" + i, expected, actual); } } } From 693865ead45bf97e1ac79b836fd59f316c891589 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Tue, 5 Jun 2018 08:51:52 +0200 Subject: [PATCH 03/13] Improve documentation of dynamic mappings. (#30952) Closes #30939 --- .../mapping/dynamic/field-mapping.asciidoc | 2 +- .../mapping/dynamic/templates.asciidoc | 21 ++++++++++++++----- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/docs/reference/mapping/dynamic/field-mapping.asciidoc b/docs/reference/mapping/dynamic/field-mapping.asciidoc index 8b2e106bfbfbc..5e0cc7e0bd6d0 100644 --- a/docs/reference/mapping/dynamic/field-mapping.asciidoc +++ b/docs/reference/mapping/dynamic/field-mapping.asciidoc @@ -135,6 +135,6 @@ PUT my_index/_doc/1 } -------------------------------------------------- // CONSOLE -<1> The `my_float` field is added as a <> field. +<1> The `my_float` field is added as a <> field. <2> The `my_integer` field is added as a <> field. diff --git a/docs/reference/mapping/dynamic/templates.asciidoc b/docs/reference/mapping/dynamic/templates.asciidoc index c715ee68f8b41..4a15436b804b4 100644 --- a/docs/reference/mapping/dynamic/templates.asciidoc +++ b/docs/reference/mapping/dynamic/templates.asciidoc @@ -46,11 +46,22 @@ name as an existing template, it will replace the old version. [[match-mapping-type]] ==== `match_mapping_type` -The `match_mapping_type` matches on the datatype detected by -<>, in other words, the datatype -that Elasticsearch thinks the field should have. Only the following datatypes -can be automatically detected: `boolean`, `date`, `double`, `long`, `object`, -`string`. It also accepts `*` to match all datatypes. +The `match_mapping_type` is the datatype detected by the json parser. Since +JSON doesn't allow to distinguish a `long` from an `integer` or a `double` from +a `float`, it will always choose the wider datatype, ie. `long` for integers +and `double` for floating-point numbers. + +The following datatypes may be automatically detected: + + - `boolean` when `true` or `false` are encountered. + - `date` when <> is enabled and a string is + found that matches any of the configured date formats. + - `double` for numbers with a decimal part. + - `long` for numbers without a decimal part. + - `object` for objects, also called hashes. + - `string` for character strings. + +`*` may also be used in order to match all datatypes. For example, if we wanted to map all integer fields as `integer` instead of `long`, and all `string` fields as both `text` and `keyword`, we From 8fbf9108b6487f02e60bf9a310725bde153811fa Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Tue, 5 Jun 2018 08:58:52 +0200 Subject: [PATCH 04/13] Docs: remove notes on sparsity. (#30905) Sparsity is less of a concern since 6.0. Closes #30833 --- docs/reference/how-to/general.asciidoc | 91 -------------------------- 1 file changed, 91 deletions(-) diff --git a/docs/reference/how-to/general.asciidoc b/docs/reference/how-to/general.asciidoc index 0900c49ce0640..5b23ee37e56d5 100644 --- a/docs/reference/how-to/general.asciidoc +++ b/docs/reference/how-to/general.asciidoc @@ -40,94 +40,3 @@ better. For instance if a user searches for two words `foo` and `bar`, a match across different chapters is probably very poor, while a match within the same paragraph is likely good. -[float] -[[sparsity]] -=== Avoid sparsity - -The data-structures behind Lucene, which Elasticsearch relies on in order to -index and store data, work best with dense data, ie. when all documents have the -same fields. This is especially true for fields that have norms enabled (which -is the case for `text` fields by default) or doc values enabled (which is the -case for numerics, `date`, `ip` and `keyword` by default). - -The reason is that Lucene internally identifies documents with so-called doc -ids, which are integers between 0 and the total number of documents in the -index. These doc ids are used for communication between the internal APIs of -Lucene: for instance searching on a term with a `match` query produces an -iterator of doc ids, and these doc ids are then used to retrieve the value of -the `norm` in order to compute a score for these documents. The way this `norm` -lookup is implemented currently is by reserving one byte for each document. -The `norm` value for a given doc id can then be retrieved by reading the -byte at index `doc_id`. While this is very efficient and helps Lucene quickly -have access to the `norm` values of every document, this has the drawback that -documents that do not have a value will also require one byte of storage. - -In practice, this means that if an index has `M` documents, norms will require -`M` bytes of storage *per field*, even for fields that only appear in a small -fraction of the documents of the index. Although slightly more complex with doc -values due to the fact that doc values have multiple ways that they can be -encoded depending on the type of field and on the actual data that the field -stores, the problem is very similar. In case you wonder: `fielddata`, which was -used in Elasticsearch pre-2.0 before being replaced with doc values, also -suffered from this issue, except that the impact was only on the memory -footprint since `fielddata` was not explicitly materialized on disk. - -Note that even though the most notable impact of sparsity is on storage -requirements, it also has an impact on indexing speed and search speed since -these bytes for documents that do not have a field still need to be written -at index time and skipped over at search time. - -It is totally fine to have a minority of sparse fields in an index. But beware -that if sparsity becomes the rule rather than the exception, then the index -will not be as efficient as it could be. - -This section mostly focused on `norms` and `doc values` because those are the -two features that are most affected by sparsity. Sparsity also affect the -efficiency of the inverted index (used to index `text`/`keyword` fields) and -dimensional points (used to index `geo_point` and numerics) but to a lesser -extent. - -Here are some recommendations that can help avoid sparsity: - -[float] -==== Avoid putting unrelated data in the same index - -You should avoid putting documents that have totally different structures into -the same index in order to avoid sparsity. It is often better to put these -documents into different indices, you could also consider giving fewer shards -to these smaller indices since they will contain fewer documents overall. - -Note that this advice does not apply in the case that you need to use -parent/child relations between your documents since this feature is only -supported on documents that live in the same index. - -[float] -==== Normalize document structures - -Even if you really need to put different kinds of documents in the same index, -maybe there are opportunities to reduce sparsity. For instance if all documents -in the index have a timestamp field but some call it `timestamp` and others -call it `creation_date`, it would help to rename it so that all documents have -the same field name for the same data. - -[float] -==== Avoid types - -Types might sound like a good way to store multiple tenants in a single index. -They are not: given that types store everything in a single index, having -multiple types that have different fields in a single index will also cause -problems due to sparsity as described above. If your types do not have very -similar mappings, you might want to consider moving them to a dedicated index. - -[float] -==== Disable `norms` and `doc_values` on sparse fields - -If none of the above recommendations apply in your case, you might want to -check whether you actually need `norms` and `doc_values` on your sparse fields. -`norms` can be disabled if producing scores is not necessary on a field, this is -typically true for fields that are only used for filtering. `doc_values` can be -disabled on fields that are neither used for sorting nor for aggregations. -Beware that this decision should not be made lightly since these parameters -cannot be changed on a live index, so you would have to reindex if you realize -that you need `norms` or `doc_values`. - From 3c725cf3e761d8ce05609acea25061d8c4e43e82 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Tue, 5 Jun 2018 08:59:17 +0200 Subject: [PATCH 05/13] Clarify docs about boolean operator precedence. (#30808) Unfortunately, the classic queryparser does not honor the usual precedence rules of boolean operators. See https://issues.apache.org/jira/browse/LUCENE-3674. --- .../query-dsl/query-string-syntax.asciidoc | 24 ++++--------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/docs/reference/query-dsl/query-string-syntax.asciidoc b/docs/reference/query-dsl/query-string-syntax.asciidoc index c73543c99a1d9..ca852ee728b5d 100644 --- a/docs/reference/query-dsl/query-string-syntax.asciidoc +++ b/docs/reference/query-dsl/query-string-syntax.asciidoc @@ -235,26 +235,10 @@ states that: * `news` must not be present * `quick` and `brown` are optional -- their presence increases the relevance -The familiar operators `AND`, `OR` and `NOT` (also written `&&`, `||` and `!`) -are also supported. However, the effects of these operators can be more -complicated than is obvious at first glance. `NOT` takes precedence over -`AND`, which takes precedence over `OR`. While the `+` and `-` only affect -the term to the right of the operator, `AND` and `OR` can affect the terms to -the left and right. - -**** -Rewriting the above query using `AND`, `OR` and `NOT` demonstrates the -complexity: - -`quick OR brown AND fox AND NOT news`:: - -This is incorrect, because `brown` is now a required term. - -`(quick OR brown) AND fox AND NOT news`:: - -This is incorrect because at least one of `quick` or `brown` is now required -and the search for those terms would be scored differently from the original -query. +The familiar boolean operators `AND`, `OR` and `NOT` (also written `&&`, `||` +and `!`) are also supported but beware that they do not honor the usual +precedence rules, so parentheses should be used whenever multiple operators are +used together. For instance the previous query could be rewritten as: `((quick AND fox) OR (brown AND fox) OR fox) AND NOT news`:: From 205194761b39e21f2c5b2b59aaae02879d7fae14 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Tue, 5 Jun 2018 09:01:12 +0200 Subject: [PATCH 06/13] Move caching of the size of a directory to `StoreDirectory`. (#30581) In spite of the existing caching, I have seen a number of nodes hot threads where one thread had been spending all its cpu on computing the size of a directory. I am proposing to move the computation of the size of the directory to `StoreDirectory` in order to skip recomputing the size of the directory if no changes have been made. This should help with users that have read-only indices, which is very common for time-based indices. --- .../common/util/SingleObjectCache.java | 5 + .../index/store/ByteSizeCachingDirectory.java | 183 ++++++++++++++++++ .../org/elasticsearch/index/store/Store.java | 54 ++---- .../store/ByteSizeCachingDirectoryTests.java | 102 ++++++++++ 4 files changed, 302 insertions(+), 42 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/index/store/ByteSizeCachingDirectory.java create mode 100644 server/src/test/java/org/elasticsearch/index/store/ByteSizeCachingDirectoryTests.java diff --git a/server/src/main/java/org/elasticsearch/common/util/SingleObjectCache.java b/server/src/main/java/org/elasticsearch/common/util/SingleObjectCache.java index f3d710dab8c79..fb0d3951aa36e 100644 --- a/server/src/main/java/org/elasticsearch/common/util/SingleObjectCache.java +++ b/server/src/main/java/org/elasticsearch/common/util/SingleObjectCache.java @@ -64,6 +64,11 @@ public T getOrRefresh() { return cached; } + /** Return the potentially stale cached entry. */ + protected final T getNoRefresh() { + return cached; + } + /** * Returns a new instance to cache */ diff --git a/server/src/main/java/org/elasticsearch/index/store/ByteSizeCachingDirectory.java b/server/src/main/java/org/elasticsearch/index/store/ByteSizeCachingDirectory.java new file mode 100644 index 0000000000000..3b0a912c2df79 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/index/store/ByteSizeCachingDirectory.java @@ -0,0 +1,183 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.store; + +import org.apache.lucene.store.Directory; +import org.apache.lucene.store.FilterDirectory; +import org.apache.lucene.store.IOContext; +import org.apache.lucene.store.IndexOutput; +import org.elasticsearch.common.lucene.store.FilterIndexOutput; +import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.common.util.SingleObjectCache; + +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.AccessDeniedException; +import java.nio.file.NoSuchFileException; + +final class ByteSizeCachingDirectory extends FilterDirectory { + + private static class SizeAndModCount { + final long size; + final long modCount; + final boolean pendingWrite; + + SizeAndModCount(long length, long modCount, boolean pendingWrite) { + this.size = length; + this.modCount = modCount; + this.pendingWrite = pendingWrite; + } + } + + private static long estimateSizeInBytes(Directory directory) throws IOException { + long estimatedSize = 0; + String[] files = directory.listAll(); + for (String file : files) { + try { + estimatedSize += directory.fileLength(file); + } catch (NoSuchFileException | FileNotFoundException | AccessDeniedException e) { + // ignore, the file is not there no more; on Windows, if one thread concurrently deletes a file while + // calling Files.size, you can also sometimes hit AccessDeniedException + } + } + return estimatedSize; + } + + private final SingleObjectCache size; + // Both these variables need to be accessed under `this` lock. + private long modCount = 0; + private long numOpenOutputs = 0; + + ByteSizeCachingDirectory(Directory in, TimeValue refreshInterval) { + super(in); + size = new SingleObjectCache(refreshInterval, new SizeAndModCount(0L, -1L, true)) { + @Override + protected SizeAndModCount refresh() { + // It is ok for the size of the directory to be more recent than + // the mod count, we would just recompute the size of the + // directory on the next call as well. However the opposite + // would be bad as we would potentially have a stale cache + // entry for a long time. So we fetch the values of modCount and + // numOpenOutputs BEFORE computing the size of the directory. + final long modCount; + final boolean pendingWrite; + synchronized(ByteSizeCachingDirectory.this) { + modCount = ByteSizeCachingDirectory.this.modCount; + pendingWrite = ByteSizeCachingDirectory.this.numOpenOutputs != 0; + } + final long size; + try { + // Compute this OUTSIDE of the lock + size = estimateSizeInBytes(getDelegate()); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + return new SizeAndModCount(size, modCount, pendingWrite); + } + + @Override + protected boolean needsRefresh() { + if (super.needsRefresh() == false) { + // The size was computed recently, don't recompute + return false; + } + SizeAndModCount cached = getNoRefresh(); + if (cached.pendingWrite) { + // The cached entry was generated while there were pending + // writes, so the size might be stale: recompute. + return true; + } + synchronized(ByteSizeCachingDirectory.this) { + // If there are pending writes or if new files have been + // written/deleted since last time: recompute + return numOpenOutputs != 0 || cached.modCount != modCount; + } + } + }; + } + + /** Return the cumulative size of all files in this directory. */ + long estimateSizeInBytes() throws IOException { + try { + return size.getOrRefresh().size; + } catch (UncheckedIOException e) { + // we wrapped in the cache and unwrap here + throw e.getCause(); + } + } + + @Override + public IndexOutput createOutput(String name, IOContext context) throws IOException { + return wrapIndexOutput(super.createOutput(name, context)); + } + + @Override + public IndexOutput createTempOutput(String prefix, String suffix, IOContext context) throws IOException { + return wrapIndexOutput(super.createTempOutput(prefix, suffix, context)); + } + + private IndexOutput wrapIndexOutput(IndexOutput out) { + synchronized (this) { + numOpenOutputs++; + } + return new FilterIndexOutput(out.toString(), out) { + @Override + public void writeBytes(byte[] b, int length) throws IOException { + // Don't write to atomicXXX here since it might be called in + // tight loops and memory barriers are costly + super.writeBytes(b, length); + } + + @Override + public void writeByte(byte b) throws IOException { + // Don't write to atomicXXX here since it might be called in + // tight loops and memory barriers are costly + super.writeByte(b); + } + + @Override + public void close() throws IOException { + // Close might cause some data to be flushed from in-memory buffers, so + // increment the modification counter too. + try { + super.close(); + } finally { + synchronized (this) { + numOpenOutputs--; + modCount++; + } + } + } + }; + } + + @Override + public void deleteFile(String name) throws IOException { + try { + super.deleteFile(name); + } finally { + synchronized (this) { + modCount++; + } + } + } + +} diff --git a/server/src/main/java/org/elasticsearch/index/store/Store.java b/server/src/main/java/org/elasticsearch/index/store/Store.java index 9bcd60d676231..dcc25dfe057e5 100644 --- a/server/src/main/java/org/elasticsearch/index/store/Store.java +++ b/server/src/main/java/org/elasticsearch/index/store/Store.java @@ -50,7 +50,6 @@ import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.BytesRefBuilder; import org.apache.lucene.util.Version; -import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.bytes.BytesReference; @@ -67,7 +66,6 @@ import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; -import org.elasticsearch.common.util.SingleObjectCache; import org.elasticsearch.common.util.concurrent.AbstractRefCounted; import org.elasticsearch.common.util.concurrent.RefCounted; import org.elasticsearch.common.util.iterable.Iterables; @@ -91,7 +89,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.PrintStream; -import java.nio.file.AccessDeniedException; import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.util.ArrayList; @@ -146,7 +143,6 @@ public class Store extends AbstractIndexShardComponent implements Closeable, Ref private final ReentrantReadWriteLock metadataLock = new ReentrantReadWriteLock(); private final ShardLock shardLock; private final OnClose onClose; - private final SingleObjectCache statsCache; private final AbstractRefCounted refCounter = new AbstractRefCounted("store") { @Override @@ -164,12 +160,13 @@ public Store(ShardId shardId, IndexSettings indexSettings, DirectoryService dire OnClose onClose) throws IOException { super(shardId, indexSettings); final Settings settings = indexSettings.getSettings(); - this.directory = new StoreDirectory(directoryService.newDirectory(), Loggers.getLogger("index.store.deletes", settings, shardId)); - this.shardLock = shardLock; - this.onClose = onClose; + Directory dir = directoryService.newDirectory(); final TimeValue refreshInterval = indexSettings.getValue(INDEX_STORE_STATS_REFRESH_INTERVAL_SETTING); - this.statsCache = new StoreStatsCache(refreshInterval, directory); logger.debug("store stats are refreshed with refresh_interval [{}]", refreshInterval); + ByteSizeCachingDirectory sizeCachingDir = new ByteSizeCachingDirectory(dir, refreshInterval); + this.directory = new StoreDirectory(sizeCachingDir, Loggers.getLogger("index.store.deletes", settings, shardId)); + this.shardLock = shardLock; + this.onClose = onClose; assert onClose != null; assert shardLock != null; @@ -377,7 +374,7 @@ public void exorciseIndex(CheckIndex.Status status) throws IOException { public StoreStats stats() throws IOException { ensureOpen(); - return statsCache.getOrRefresh(); + return new StoreStats(directory.estimateSize()); } /** @@ -731,11 +728,16 @@ static final class StoreDirectory extends FilterDirectory { private final Logger deletesLogger; - StoreDirectory(Directory delegateDirectory, Logger deletesLogger) { + StoreDirectory(ByteSizeCachingDirectory delegateDirectory, Logger deletesLogger) { super(delegateDirectory); this.deletesLogger = deletesLogger; } + /** Estimate the cumulative size of all files in this directory in bytes. */ + long estimateSize() throws IOException { + return ((ByteSizeCachingDirectory) getDelegate()).estimateSizeInBytes(); + } + @Override public void close() { assert false : "Nobody should close this directory except of the Store itself"; @@ -1428,38 +1430,6 @@ public void accept(ShardLock Lock) { }; } - private static class StoreStatsCache extends SingleObjectCache { - private final Directory directory; - - StoreStatsCache(TimeValue refreshInterval, Directory directory) throws IOException { - super(refreshInterval, new StoreStats(estimateSize(directory))); - this.directory = directory; - } - - @Override - protected StoreStats refresh() { - try { - return new StoreStats(estimateSize(directory)); - } catch (IOException ex) { - throw new ElasticsearchException("failed to refresh store stats", ex); - } - } - - private static long estimateSize(Directory directory) throws IOException { - long estimatedSize = 0; - String[] files = directory.listAll(); - for (String file : files) { - try { - estimatedSize += directory.fileLength(file); - } catch (NoSuchFileException | FileNotFoundException | AccessDeniedException e) { - // ignore, the file is not there no more; on Windows, if one thread concurrently deletes a file while - // calling Files.size, you can also sometimes hit AccessDeniedException - } - } - return estimatedSize; - } - } - /** * creates an empty lucene index and a corresponding empty translog. Any existing data will be deleted. */ diff --git a/server/src/test/java/org/elasticsearch/index/store/ByteSizeCachingDirectoryTests.java b/server/src/test/java/org/elasticsearch/index/store/ByteSizeCachingDirectoryTests.java new file mode 100644 index 0000000000000..25d783d25315f --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/store/ByteSizeCachingDirectoryTests.java @@ -0,0 +1,102 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.store; + +import org.apache.lucene.store.Directory; +import org.apache.lucene.store.FilterDirectory; +import org.apache.lucene.store.IOContext; +import org.apache.lucene.store.IndexOutput; +import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.test.ESTestCase; + +import java.io.IOException; + +public class ByteSizeCachingDirectoryTests extends ESTestCase { + + private static class LengthCountingDirectory extends FilterDirectory { + + int numFileLengthCalls; + + LengthCountingDirectory(Directory in) { + super(in); + } + + @Override + public long fileLength(String name) throws IOException { + numFileLengthCalls++; + return super.fileLength(name); + } + } + + public void testBasics() throws IOException { + try (Directory dir = newDirectory()) { + try (IndexOutput out = dir.createOutput("quux", IOContext.DEFAULT)) { + out.writeBytes(new byte[11], 11); + } + LengthCountingDirectory countingDir = new LengthCountingDirectory(dir); + + ByteSizeCachingDirectory cachingDir = new ByteSizeCachingDirectory(countingDir, new TimeValue(0)); + assertEquals(11, cachingDir.estimateSizeInBytes()); + assertEquals(11, cachingDir.estimateSizeInBytes()); + assertEquals(1, countingDir.numFileLengthCalls); + + try (IndexOutput out = cachingDir.createOutput("foo", IOContext.DEFAULT)) { + out.writeBytes(new byte[5], 5); + + cachingDir.estimateSizeInBytes(); + // +2 because there are 3 files + assertEquals(3, countingDir.numFileLengthCalls); + // An index output is open so no caching + cachingDir.estimateSizeInBytes(); + assertEquals(5, countingDir.numFileLengthCalls); + } + + assertEquals(16, cachingDir.estimateSizeInBytes()); + assertEquals(7, countingDir.numFileLengthCalls); + assertEquals(16, cachingDir.estimateSizeInBytes()); + assertEquals(7, countingDir.numFileLengthCalls); + + try (IndexOutput out = cachingDir.createTempOutput("bar", "baz", IOContext.DEFAULT)) { + out.writeBytes(new byte[4], 4); + + cachingDir.estimateSizeInBytes(); + assertEquals(10, countingDir.numFileLengthCalls); + // An index output is open so no caching + cachingDir.estimateSizeInBytes(); + assertEquals(13, countingDir.numFileLengthCalls); + } + + assertEquals(20, cachingDir.estimateSizeInBytes()); + // +3 because there are 3 files + assertEquals(16, countingDir.numFileLengthCalls); + assertEquals(20, cachingDir.estimateSizeInBytes()); + assertEquals(16, countingDir.numFileLengthCalls); + + cachingDir.deleteFile("foo"); + + assertEquals(15, cachingDir.estimateSizeInBytes()); + // +2 because there are 2 files now + assertEquals(18, countingDir.numFileLengthCalls); + assertEquals(15, cachingDir.estimateSizeInBytes()); + assertEquals(18, countingDir.numFileLengthCalls); + } + } + +} From 1889e8e742dd0f41c3fb5956d7324f463eb81fcb Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Tue, 5 Jun 2018 13:00:43 +0200 Subject: [PATCH 07/13] Add BlobContainer.writeBlobAtomic() (#30902) This commit adds a new writeBlobAtomic() method to the BlobContainer interface that can be implemented by repository implementations which support atomic writes operations. When the BlobContainer implementation does not provide a specific implementation of writeBlobAtomic(), then the writeBlob() method is used. Related to #30680 --- .../resources/checkstyle_suppressions.xml | 2 - .../common/blobstore/BlobContainer.java | 23 ++++++ .../common/blobstore/fs/FsBlobContainer.java | 48 ++++++++++++- .../blobstore/BlobStoreRepository.java | 19 ++--- .../blobstore/ChecksumBlobStoreFormat.java | 71 ++++++------------- .../blobstore/fs/FsBlobContainerTests.java | 40 +++++++++++ .../{ => fs}/FsBlobStoreContainerTests.java | 16 +++-- .../blobstore/{ => fs}/FsBlobStoreTests.java | 17 +++-- .../AbstractSnapshotIntegTestCase.java | 2 +- .../snapshots/BlobStoreFormatIT.java | 42 +---------- .../mockstore/BlobContainerWrapper.java | 10 +++ .../snapshots/mockstore/MockRepository.java | 63 ++++++++++++---- .../ESBlobStoreContainerTestCase.java | 6 +- 13 files changed, 222 insertions(+), 137 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/common/blobstore/fs/FsBlobContainerTests.java rename server/src/test/java/org/elasticsearch/common/blobstore/{ => fs}/FsBlobStoreContainerTests.java (75%) rename server/src/test/java/org/elasticsearch/common/blobstore/{ => fs}/FsBlobStoreTests.java (84%) diff --git a/buildSrc/src/main/resources/checkstyle_suppressions.xml b/buildSrc/src/main/resources/checkstyle_suppressions.xml index b05a02af1ddd3..09142139b311d 100644 --- a/buildSrc/src/main/resources/checkstyle_suppressions.xml +++ b/buildSrc/src/main/resources/checkstyle_suppressions.xml @@ -524,8 +524,6 @@ - - diff --git a/server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java b/server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java index 6b9992e7e4c3a..7e3a385443f84 100644 --- a/server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java +++ b/server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java @@ -74,6 +74,29 @@ public interface BlobContainer { */ void writeBlob(String blobName, InputStream inputStream, long blobSize) throws IOException; + /** + * Reads blob content from the input stream and writes it to the container in a new blob with the given name, + * using an atomic write operation if the implementation supports it. When the BlobContainer implementation + * does not provide a specific implementation of writeBlobAtomic(String, InputStream, long), then + * the {@link #writeBlob(String, InputStream, long)} method is used. + * + * This method assumes the container does not already contain a blob of the same blobName. If a blob by the + * same name already exists, the operation will fail and an {@link IOException} will be thrown. + * + * @param blobName + * The name of the blob to write the contents of the input stream to. + * @param inputStream + * The input stream from which to retrieve the bytes to write to the blob. + * @param blobSize + * The size of the blob to be written, in bytes. It is implementation dependent whether + * this value is used in writing the blob to the repository. + * @throws FileAlreadyExistsException if a blob by the same name already exists + * @throws IOException if the input stream could not be read, or the target blob could not be written to. + */ + default void writeBlobAtomic(final String blobName, final InputStream inputStream, final long blobSize) throws IOException { + writeBlob(blobName, inputStream, blobSize); + } + /** * Deletes a blob with giving name, if the blob exists. If the blob does not exist, * this method throws a NoSuchFileException. diff --git a/server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java b/server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java index a9600681d1605..6f1df0011b147 100644 --- a/server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java +++ b/server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java @@ -19,11 +19,12 @@ package org.elasticsearch.common.blobstore.fs; -import org.elasticsearch.core.internal.io.IOUtils; +import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.blobstore.BlobMetaData; import org.elasticsearch.common.blobstore.BlobPath; import org.elasticsearch.common.blobstore.support.AbstractBlobContainer; import org.elasticsearch.common.blobstore.support.PlainBlobMetaData; +import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.core.internal.io.Streams; import java.io.BufferedInputStream; @@ -56,8 +57,9 @@ */ public class FsBlobContainer extends AbstractBlobContainer { - protected final FsBlobStore blobStore; + private static final String TEMP_FILE_PREFIX = "pending-"; + protected final FsBlobStore blobStore; protected final Path path; public FsBlobContainer(FsBlobStore blobStore, BlobPath blobPath, Path path) { @@ -131,6 +133,48 @@ public void writeBlob(String blobName, InputStream inputStream, long blobSize) t IOUtils.fsync(path, true); } + @Override + public void writeBlobAtomic(final String blobName, final InputStream inputStream, final long blobSize) throws IOException { + final String tempBlob = tempBlobName(blobName); + final Path tempBlobPath = path.resolve(tempBlob); + try { + try (OutputStream outputStream = Files.newOutputStream(tempBlobPath, StandardOpenOption.CREATE_NEW)) { + Streams.copy(inputStream, outputStream); + } + IOUtils.fsync(tempBlobPath, false); + + final Path blobPath = path.resolve(blobName); + // If the target file exists then Files.move() behaviour is implementation specific + // the existing file might be replaced or this method fails by throwing an IOException. + if (Files.exists(blobPath)) { + throw new FileAlreadyExistsException("blob [" + blobPath + "] already exists, cannot overwrite"); + } + Files.move(tempBlobPath, blobPath, StandardCopyOption.ATOMIC_MOVE); + } catch (IOException ex) { + try { + deleteBlobIgnoringIfNotExists(tempBlob); + } catch (IOException e) { + ex.addSuppressed(e); + } + throw ex; + } finally { + IOUtils.fsync(path, true); + } + } + + public static String tempBlobName(final String blobName) { + return "pending-" + blobName + "-" + UUIDs.randomBase64UUID(); + } + + /** + * Returns true if the blob is a leftover temporary blob. + * + * The temporary blobs might be left after failed atomic write operation. + */ + public static boolean isTempBlobName(final String blobName) { + return blobName.startsWith(TEMP_FILE_PREFIX); + } + @Override public void move(String source, String target) throws IOException { Path sourcePath = path.resolve(source); diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index d6115cae1a6f0..d23f8e10920ed 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -50,6 +50,7 @@ import org.elasticsearch.common.blobstore.BlobMetaData; import org.elasticsearch.common.blobstore.BlobPath; import org.elasticsearch.common.blobstore.BlobStore; +import org.elasticsearch.common.blobstore.fs.FsBlobContainer; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.collect.Tuple; @@ -555,10 +556,8 @@ public String startVerification() { String blobName = "master.dat"; BytesArray bytes = new BytesArray(testBytes); try (InputStream stream = bytes.streamInput()) { - testContainer.writeBlob(blobName + "-temp", stream, bytes.length()); + testContainer.writeBlobAtomic(blobName, stream, bytes.length()); } - // Make sure that move is supported - testContainer.move(blobName + "-temp", blobName); return seed; } } catch (IOException exp) { @@ -774,18 +773,8 @@ private long listBlobsToGetLatestIndexId() throws IOException { } private void writeAtomic(final String blobName, final BytesReference bytesRef) throws IOException { - final String tempBlobName = "pending-" + blobName + "-" + UUIDs.randomBase64UUID(); try (InputStream stream = bytesRef.streamInput()) { - snapshotsBlobContainer.writeBlob(tempBlobName, stream, bytesRef.length()); - snapshotsBlobContainer.move(tempBlobName, blobName); - } catch (IOException ex) { - // temporary blob creation or move failed - try cleaning up - try { - snapshotsBlobContainer.deleteBlobIgnoringIfNotExists(tempBlobName); - } catch (IOException e) { - ex.addSuppressed(e); - } - throw ex; + snapshotsBlobContainer.writeBlobAtomic(blobName, stream, bytesRef.length()); } } @@ -955,7 +944,7 @@ protected void finalize(final List snapshots, // Delete temporary index files first, as we might otherwise fail in the next step creating the new index file if an earlier // attempt to write an index file with this generation failed mid-way after creating the temporary file. for (final String blobName : blobs.keySet()) { - if (indexShardSnapshotsFormat.isTempBlobName(blobName)) { + if (FsBlobContainer.isTempBlobName(blobName)) { try { blobContainer.deleteBlobIgnoringIfNotExists(blobName); } catch (IOException e) { diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/ChecksumBlobStoreFormat.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/ChecksumBlobStoreFormat.java index 8c8139d5abd6a..df9b41ba87299 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/ChecksumBlobStoreFormat.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/ChecksumBlobStoreFormat.java @@ -23,6 +23,7 @@ import org.apache.lucene.index.IndexFormatTooNewException; import org.apache.lucene.index.IndexFormatTooOldException; import org.apache.lucene.store.OutputStreamIndexOutput; +import org.elasticsearch.common.CheckedConsumer; import org.elasticsearch.common.CheckedFunction; import org.elasticsearch.common.blobstore.BlobContainer; import org.elasticsearch.common.bytes.BytesArray; @@ -52,8 +53,6 @@ */ public class ChecksumBlobStoreFormat extends BlobStoreFormat { - private static final String TEMP_FILE_PREFIX = "pending-"; - private static final XContentType DEFAULT_X_CONTENT_TYPE = XContentType.SMILE; // The format version @@ -120,7 +119,7 @@ public T readBlob(BlobContainer blobContainer, String blobName) throws IOExcepti } /** - * Writes blob in atomic manner with resolving the blob name using {@link #blobName} and {@link #tempBlobName} methods. + * Writes blob in atomic manner with resolving the blob name using {@link #blobName} method. *

* The blob will be compressed and checksum will be written if required. * @@ -131,20 +130,12 @@ public T readBlob(BlobContainer blobContainer, String blobName) throws IOExcepti * @param name blob name */ public void writeAtomic(T obj, BlobContainer blobContainer, String name) throws IOException { - String blobName = blobName(name); - String tempBlobName = tempBlobName(name); - writeBlob(obj, blobContainer, tempBlobName); - try { - blobContainer.move(tempBlobName, blobName); - } catch (IOException ex) { - // Move failed - try cleaning up - try { - blobContainer.deleteBlob(tempBlobName); - } catch (Exception e) { - ex.addSuppressed(e); + final String blobName = blobName(name); + writeTo(obj, blobName, bytesArray -> { + try (InputStream stream = bytesArray.streamInput()) { + blobContainer.writeBlobAtomic(blobName, stream, bytesArray.length()); } - throw ex; - } + }); } /** @@ -157,51 +148,35 @@ public void writeAtomic(T obj, BlobContainer blobContainer, String name) throws * @param name blob name */ public void write(T obj, BlobContainer blobContainer, String name) throws IOException { - String blobName = blobName(name); - writeBlob(obj, blobContainer, blobName); + final String blobName = blobName(name); + writeTo(obj, blobName, bytesArray -> { + try (InputStream stream = bytesArray.streamInput()) { + blobContainer.writeBlob(blobName, stream, bytesArray.length()); + } + }); } - /** - * Writes blob in atomic manner without resolving the blobName using using {@link #blobName} method. - *

- * The blob will be compressed and checksum will be written if required. - * - * @param obj object to be serialized - * @param blobContainer blob container - * @param blobName blob name - */ - protected void writeBlob(T obj, BlobContainer blobContainer, String blobName) throws IOException { - BytesReference bytes = write(obj); - try (ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream()) { + private void writeTo(final T obj, final String blobName, final CheckedConsumer consumer) throws IOException { + final BytesReference bytes = write(obj); + try (ByteArrayOutputStream outputStream = new ByteArrayOutputStream()) { final String resourceDesc = "ChecksumBlobStoreFormat.writeBlob(blob=\"" + blobName + "\")"; - try (OutputStreamIndexOutput indexOutput = new OutputStreamIndexOutput(resourceDesc, blobName, byteArrayOutputStream, BUFFER_SIZE)) { + try (OutputStreamIndexOutput indexOutput = new OutputStreamIndexOutput(resourceDesc, blobName, outputStream, BUFFER_SIZE)) { CodecUtil.writeHeader(indexOutput, codec, VERSION); try (OutputStream indexOutputOutputStream = new IndexOutputOutputStream(indexOutput) { @Override public void close() throws IOException { // this is important since some of the XContentBuilders write bytes on close. // in order to write the footer we need to prevent closing the actual index input. - } }) { + } + }) { bytes.writeTo(indexOutputOutputStream); } CodecUtil.writeFooter(indexOutput); } - BytesArray bytesArray = new BytesArray(byteArrayOutputStream.toByteArray()); - try (InputStream stream = bytesArray.streamInput()) { - blobContainer.writeBlob(blobName, stream, bytesArray.length()); - } + consumer.accept(new BytesArray(outputStream.toByteArray())); } } - /** - * Returns true if the blob is a leftover temporary blob. - * - * The temporary blobs might be left after failed atomic write operation. - */ - public boolean isTempBlobName(String blobName) { - return blobName.startsWith(ChecksumBlobStoreFormat.TEMP_FILE_PREFIX); - } - protected BytesReference write(T obj) throws IOException { try (BytesStreamOutput bytesStreamOutput = new BytesStreamOutput()) { if (compress) { @@ -222,10 +197,4 @@ protected void write(T obj, StreamOutput streamOutput) throws IOException { builder.endObject(); } } - - - protected String tempBlobName(String name) { - return TEMP_FILE_PREFIX + String.format(Locale.ROOT, blobNameFormat, name); - } - } diff --git a/server/src/test/java/org/elasticsearch/common/blobstore/fs/FsBlobContainerTests.java b/server/src/test/java/org/elasticsearch/common/blobstore/fs/FsBlobContainerTests.java new file mode 100644 index 0000000000000..c603eda906cae --- /dev/null +++ b/server/src/test/java/org/elasticsearch/common/blobstore/fs/FsBlobContainerTests.java @@ -0,0 +1,40 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.common.blobstore.fs; + +import org.elasticsearch.test.ESTestCase; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.startsWith; + +public class FsBlobContainerTests extends ESTestCase { + + public void testTempBlobName() { + final String blobName = randomAlphaOfLengthBetween(1, 20); + final String tempBlobName = FsBlobContainer.tempBlobName(blobName); + assertThat(tempBlobName, startsWith("pending-")); + assertThat(tempBlobName, containsString(blobName)); + } + + public void testIsTempBlobName() { + final String tempBlobName = FsBlobContainer.tempBlobName(randomAlphaOfLengthBetween(1, 20)); + assertThat(FsBlobContainer.isTempBlobName(tempBlobName), is(true)); + } +} diff --git a/server/src/test/java/org/elasticsearch/common/blobstore/FsBlobStoreContainerTests.java b/server/src/test/java/org/elasticsearch/common/blobstore/fs/FsBlobStoreContainerTests.java similarity index 75% rename from server/src/test/java/org/elasticsearch/common/blobstore/FsBlobStoreContainerTests.java rename to server/src/test/java/org/elasticsearch/common/blobstore/fs/FsBlobStoreContainerTests.java index b08b81db11aeb..9230cded82b1d 100644 --- a/server/src/test/java/org/elasticsearch/common/blobstore/FsBlobStoreContainerTests.java +++ b/server/src/test/java/org/elasticsearch/common/blobstore/fs/FsBlobStoreContainerTests.java @@ -16,23 +16,27 @@ * specific language governing permissions and limitations * under the License. */ -package org.elasticsearch.common.blobstore; +package org.elasticsearch.common.blobstore.fs; import org.apache.lucene.util.LuceneTestCase; -import org.elasticsearch.common.blobstore.fs.FsBlobStore; +import org.elasticsearch.common.blobstore.BlobStore; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeUnit; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.repositories.ESBlobStoreContainerTestCase; import java.io.IOException; -import java.nio.file.Path; @LuceneTestCase.SuppressFileSystems("ExtrasFS") public class FsBlobStoreContainerTests extends ESBlobStoreContainerTestCase { + protected BlobStore newBlobStore() throws IOException { - Path tempDir = createTempDir(); - Settings settings = randomBoolean() ? Settings.EMPTY : Settings.builder().put("buffer_size", new ByteSizeValue(randomIntBetween(1, 100), ByteSizeUnit.KB)).build(); - return new FsBlobStore(settings, tempDir); + final Settings settings; + if (randomBoolean()) { + settings = Settings.builder().put("buffer_size", new ByteSizeValue(randomIntBetween(1, 100), ByteSizeUnit.KB)).build(); + } else { + settings = Settings.EMPTY; + } + return new FsBlobStore(settings, createTempDir()); } } diff --git a/server/src/test/java/org/elasticsearch/common/blobstore/FsBlobStoreTests.java b/server/src/test/java/org/elasticsearch/common/blobstore/fs/FsBlobStoreTests.java similarity index 84% rename from server/src/test/java/org/elasticsearch/common/blobstore/FsBlobStoreTests.java rename to server/src/test/java/org/elasticsearch/common/blobstore/fs/FsBlobStoreTests.java index 8b9021cae9370..59e4ffd7927ca 100644 --- a/server/src/test/java/org/elasticsearch/common/blobstore/FsBlobStoreTests.java +++ b/server/src/test/java/org/elasticsearch/common/blobstore/fs/FsBlobStoreTests.java @@ -16,10 +16,12 @@ * specific language governing permissions and limitations * under the License. */ -package org.elasticsearch.common.blobstore; +package org.elasticsearch.common.blobstore.fs; import org.apache.lucene.util.LuceneTestCase; -import org.elasticsearch.common.blobstore.fs.FsBlobStore; +import org.elasticsearch.common.blobstore.BlobContainer; +import org.elasticsearch.common.blobstore.BlobPath; +import org.elasticsearch.common.blobstore.BlobStore; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.ByteSizeUnit; @@ -32,10 +34,15 @@ @LuceneTestCase.SuppressFileSystems("ExtrasFS") public class FsBlobStoreTests extends ESBlobStoreTestCase { + protected BlobStore newBlobStore() throws IOException { - Path tempDir = createTempDir(); - Settings settings = randomBoolean() ? Settings.EMPTY : Settings.builder().put("buffer_size", new ByteSizeValue(randomIntBetween(1, 100), ByteSizeUnit.KB)).build(); - return new FsBlobStore(settings, tempDir); + final Settings settings; + if (randomBoolean()) { + settings = Settings.builder().put("buffer_size", new ByteSizeValue(randomIntBetween(1, 100), ByteSizeUnit.KB)).build(); + } else { + settings = Settings.EMPTY; + } + return new FsBlobStore(settings, createTempDir()); } public void testReadOnly() throws Exception { diff --git a/server/src/test/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java b/server/src/test/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java index 45110ee6a2d15..23c56688e00b4 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java +++ b/server/src/test/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java @@ -94,7 +94,7 @@ public void waitForBlock(String node, String repository, TimeValue timeout) thro } Thread.sleep(100); } - fail("Timeout!!!"); + fail("Timeout waiting for node [" + node + "] to be blocked"); } public SnapshotInfo waitForCompletion(String repository, String snapshotName, TimeValue timeout) throws InterruptedException { diff --git a/server/src/test/java/org/elasticsearch/snapshots/BlobStoreFormatIT.java b/server/src/test/java/org/elasticsearch/snapshots/BlobStoreFormatIT.java index 65926234d45c0..70be72989cf95 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/BlobStoreFormatIT.java +++ b/server/src/test/java/org/elasticsearch/snapshots/BlobStoreFormatIT.java @@ -224,52 +224,16 @@ public void testAtomicWriteFailures() throws Exception { IOException writeBlobException = expectThrows(IOException.class, () -> { BlobContainer wrapper = new BlobContainerWrapper(blobContainer) { @Override - public void writeBlob(String blobName, InputStream inputStream, long blobSize) throws IOException { - throw new IOException("Exception thrown in writeBlob() for " + blobName); + public void writeBlobAtomic(String blobName, InputStream inputStream, long blobSize) throws IOException { + throw new IOException("Exception thrown in writeBlobAtomic() for " + blobName); } }; checksumFormat.writeAtomic(blobObj, wrapper, name); }); - assertEquals("Exception thrown in writeBlob() for pending-" + name, writeBlobException.getMessage()); + assertEquals("Exception thrown in writeBlobAtomic() for " + name, writeBlobException.getMessage()); assertEquals(0, writeBlobException.getSuppressed().length); } - { - IOException moveException = expectThrows(IOException.class, () -> { - BlobContainer wrapper = new BlobContainerWrapper(blobContainer) { - @Override - public void move(String sourceBlobName, String targetBlobName) throws IOException { - throw new IOException("Exception thrown in move() for " + sourceBlobName); - } - }; - checksumFormat.writeAtomic(blobObj, wrapper, name); - }); - assertEquals("Exception thrown in move() for pending-" + name, moveException.getMessage()); - assertEquals(0, moveException.getSuppressed().length); - } - { - IOException moveThenDeleteException = expectThrows(IOException.class, () -> { - BlobContainer wrapper = new BlobContainerWrapper(blobContainer) { - @Override - public void move(String sourceBlobName, String targetBlobName) throws IOException { - throw new IOException("Exception thrown in move() for " + sourceBlobName); - } - - @Override - public void deleteBlob(String blobName) throws IOException { - throw new IOException("Exception thrown in deleteBlob() for " + blobName); - } - }; - checksumFormat.writeAtomic(blobObj, wrapper, name); - }); - - assertEquals("Exception thrown in move() for pending-" + name, moveThenDeleteException.getMessage()); - assertEquals(1, moveThenDeleteException.getSuppressed().length); - - final Throwable suppressedThrowable = moveThenDeleteException.getSuppressed()[0]; - assertTrue(suppressedThrowable instanceof IOException); - assertEquals("Exception thrown in deleteBlob() for pending-" + name, suppressedThrowable.getMessage()); - } } protected BlobStore createTestBlobStore() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/snapshots/mockstore/BlobContainerWrapper.java b/server/src/test/java/org/elasticsearch/snapshots/mockstore/BlobContainerWrapper.java index 56a4a279cab62..089955d140f44 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/mockstore/BlobContainerWrapper.java +++ b/server/src/test/java/org/elasticsearch/snapshots/mockstore/BlobContainerWrapper.java @@ -53,11 +53,21 @@ public void writeBlob(String blobName, InputStream inputStream, long blobSize) t delegate.writeBlob(blobName, inputStream, blobSize); } + @Override + public void writeBlobAtomic(final String blobName, final InputStream inputStream, final long blobSize) throws IOException { + delegate.writeBlobAtomic(blobName, inputStream, blobSize); + } + @Override public void deleteBlob(String blobName) throws IOException { delegate.deleteBlob(blobName); } + @Override + public void deleteBlobIgnoringIfNotExists(final String blobName) throws IOException { + delegate.deleteBlobIgnoringIfNotExists(blobName); + } + @Override public Map listBlobs() throws IOException { return delegate.listBlobs(); diff --git a/server/src/test/java/org/elasticsearch/snapshots/mockstore/MockRepository.java b/server/src/test/java/org/elasticsearch/snapshots/mockstore/MockRepository.java index 3a5b068cd8977..5fa884adbfe62 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/mockstore/MockRepository.java +++ b/server/src/test/java/org/elasticsearch/snapshots/mockstore/MockRepository.java @@ -19,20 +19,6 @@ package org.elasticsearch.snapshots.mockstore; -import java.io.IOException; -import java.io.InputStream; -import java.io.UnsupportedEncodingException; -import java.nio.file.Path; -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.atomic.AtomicLong; - import com.carrotsearch.randomizedtesting.RandomizedContext; import org.apache.lucene.index.CorruptIndexException; import org.elasticsearch.ElasticsearchException; @@ -42,6 +28,7 @@ import org.elasticsearch.common.blobstore.BlobMetaData; import org.elasticsearch.common.blobstore.BlobPath; import org.elasticsearch.common.blobstore.BlobStore; +import org.elasticsearch.common.blobstore.fs.FsBlobContainer; import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; @@ -49,11 +36,26 @@ import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.env.Environment; import org.elasticsearch.plugins.RepositoryPlugin; -import org.elasticsearch.repositories.Repository; import org.elasticsearch.repositories.IndexId; +import org.elasticsearch.repositories.Repository; import org.elasticsearch.repositories.fs.FsRepository; import org.elasticsearch.snapshots.SnapshotId; +import java.io.IOException; +import java.io.InputStream; +import java.io.UnsupportedEncodingException; +import java.nio.file.Path; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Random; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.atomic.AtomicLong; + public class MockRepository extends FsRepository { public static class Plugin extends org.elasticsearch.plugins.Plugin implements RepositoryPlugin { @@ -325,6 +327,12 @@ public void deleteBlob(String blobName) throws IOException { super.deleteBlob(blobName); } + @Override + public void deleteBlobIgnoringIfNotExists(String blobName) throws IOException { + maybeIOExceptionOrBlock(blobName); + super.deleteBlobIgnoringIfNotExists(blobName); + } + @Override public Map listBlobs() throws IOException { maybeIOExceptionOrBlock(""); @@ -365,6 +373,31 @@ public void writeBlob(String blobName, InputStream inputStream, long blobSize) t maybeIOExceptionOrBlock(blobName); } } + + @Override + public void writeBlobAtomic(final String blobName, final InputStream inputStream, final long blobSize) throws IOException { + final Random random = RandomizedContext.current().getRandom(); + if (random.nextBoolean()) { + if ((delegate() instanceof FsBlobContainer) && (random.nextBoolean())) { + // Simulate a failure between the write and move operation in FsBlobContainer + final String tempBlobName = FsBlobContainer.tempBlobName(blobName); + super.writeBlob(tempBlobName, inputStream, blobSize); + maybeIOExceptionOrBlock(blobName); + final FsBlobContainer fsBlobContainer = (FsBlobContainer) delegate(); + fsBlobContainer.move(tempBlobName, blobName); + } else { + // Atomic write since it is potentially supported + // by the delegating blob container + maybeIOExceptionOrBlock(blobName); + super.writeBlobAtomic(blobName, inputStream, blobSize); + } + } else { + // Simulate a non-atomic write since many blob container + // implementations does not support atomic write + maybeIOExceptionOrBlock(blobName); + super.writeBlob(blobName, inputStream, blobSize); + } + } } } } diff --git a/test/framework/src/main/java/org/elasticsearch/repositories/ESBlobStoreContainerTestCase.java b/test/framework/src/main/java/org/elasticsearch/repositories/ESBlobStoreContainerTestCase.java index 743be6d1bcb01..df2024de445c1 100644 --- a/test/framework/src/main/java/org/elasticsearch/repositories/ESBlobStoreContainerTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/repositories/ESBlobStoreContainerTestCase.java @@ -158,7 +158,11 @@ public void testVerifyOverwriteFails() throws IOException { protected void writeBlob(final BlobContainer container, final String blobName, final BytesArray bytesArray) throws IOException { try (InputStream stream = bytesArray.streamInput()) { - container.writeBlob(blobName, stream, bytesArray.length()); + if (randomBoolean()) { + container.writeBlob(blobName, stream, bytesArray.length()); + } else { + container.writeBlobAtomic(blobName, stream, bytesArray.length()); + } } } From 97b3b2d8adfeae1df4322b58563e4ec25d0ba369 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Tue, 5 Jun 2018 13:43:04 +0200 Subject: [PATCH 08/13] Only auto-update license signature if all nodes ready (#30859) Allows rolling restart from 6.3 to 6.4. Relates to #30731 and #30251 --- .../elasticsearch/license/LicenseService.java | 3 ++- .../elasticsearch/license/LicenseUtils.java | 25 +++++++++++++++++-- .../license/SelfGeneratedLicense.java | 5 ++-- .../license/StartBasicClusterTask.java | 2 +- .../license/StartTrialClusterTask.java | 2 +- .../StartupSelfGeneratedLicenseTask.java | 8 +++--- .../license/LicenseRegistrationTests.java | 4 +-- .../license/LicenseSerializationTests.java | 2 +- .../LicensesMetaDataSerializationTests.java | 2 +- .../license/SelfGeneratedLicenseTests.java | 7 +++--- .../xpack/security/Security.java | 17 ++++++++++--- .../xpack/security/SecurityTests.java | 16 +++++++++++- 12 files changed, 70 insertions(+), 23 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/LicenseService.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/LicenseService.java index 99e6a10ad92de..40c694cedb764 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/LicenseService.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/LicenseService.java @@ -411,7 +411,8 @@ public void clusterChanged(ClusterChangedEvent event) { // auto-generate license if no licenses ever existed or if the current license is basic and // needs extended or if the license signature needs to be updated. this will trigger a subsequent cluster changed event if (currentClusterState.getNodes().isLocalNodeElectedMaster() && - (noLicense || LicenseUtils.licenseNeedsExtended(currentLicense) || LicenseUtils.signatureNeedsUpdate(currentLicense))) { + (noLicense || LicenseUtils.licenseNeedsExtended(currentLicense) || + LicenseUtils.signatureNeedsUpdate(currentLicense, currentClusterState.nodes()))) { registerOrUpdateSelfGeneratedLicense(); } } else if (logger.isDebugEnabled()) { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/LicenseUtils.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/LicenseUtils.java index 8fcdc05bcf986..4c8a558682b13 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/LicenseUtils.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/LicenseUtils.java @@ -6,8 +6,12 @@ package org.elasticsearch.license; import org.elasticsearch.ElasticsearchSecurityException; +import org.elasticsearch.Version; +import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.rest.RestStatus; +import java.util.stream.StreamSupport; + public class LicenseUtils { public static final String EXPIRED_FEATURE_METADATA = "es.license.expired.feature"; @@ -42,8 +46,25 @@ public static boolean licenseNeedsExtended(License license) { * Checks if the signature of a self generated license with older version needs to be * recreated with the new key */ - public static boolean signatureNeedsUpdate(License license) { + public static boolean signatureNeedsUpdate(License license, DiscoveryNodes currentNodes) { + assert License.VERSION_CRYPTO_ALGORITHMS == License.VERSION_CURRENT : "update this method when adding a new version"; + return ("basic".equals(license.type()) || "trial".equals(license.type())) && - (license.version() < License.VERSION_CRYPTO_ALGORITHMS); + // only upgrade signature when all nodes are ready to deserialize the new signature + (license.version() < License.VERSION_CRYPTO_ALGORITHMS && + compatibleLicenseVersion(currentNodes) == License.VERSION_CRYPTO_ALGORITHMS + ); + } + + public static int compatibleLicenseVersion(DiscoveryNodes currentNodes) { + assert License.VERSION_CRYPTO_ALGORITHMS == License.VERSION_CURRENT : "update this method when adding a new version"; + + if (StreamSupport.stream(currentNodes.spliterator(), false) + .allMatch(node -> node.getVersion().onOrAfter(Version.V_6_4_0))) { + // License.VERSION_CRYPTO_ALGORITHMS was introduced in 6.4.0 + return License.VERSION_CRYPTO_ALGORITHMS; + } else { + return License.VERSION_START_DATE; + } } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/SelfGeneratedLicense.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/SelfGeneratedLicense.java index 0bc49d517cd92..fb9b167d3db52 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/SelfGeneratedLicense.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/SelfGeneratedLicense.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.license; +import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; @@ -26,8 +27,8 @@ class SelfGeneratedLicense { - public static License create(License.Builder specBuilder) { - return create(specBuilder, License.VERSION_CURRENT); + public static License create(License.Builder specBuilder, DiscoveryNodes currentNodes) { + return create(specBuilder, LicenseUtils.compatibleLicenseVersion(currentNodes)); } public static License create(License.Builder specBuilder, int version) { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartBasicClusterTask.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartBasicClusterTask.java index 0cf949a69906f..468f1799a07b9 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartBasicClusterTask.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartBasicClusterTask.java @@ -73,7 +73,7 @@ public ClusterState execute(ClusterState currentState) throws Exception { .issueDate(issueDate) .type("basic") .expiryDate(LicenseService.BASIC_SELF_GENERATED_LICENSE_EXPIRATION_MILLIS); - License selfGeneratedLicense = SelfGeneratedLicense.create(specBuilder); + License selfGeneratedLicense = SelfGeneratedLicense.create(specBuilder, currentState.nodes()); if (request.isAcknowledged() == false && currentLicense != null) { Map ackMessages = LicenseService.getAckMessages(selfGeneratedLicense, currentLicense); if (ackMessages.isEmpty() == false) { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartTrialClusterTask.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartTrialClusterTask.java index 5c5c03151ba26..2bf0555fde111 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartTrialClusterTask.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartTrialClusterTask.java @@ -82,7 +82,7 @@ public ClusterState execute(ClusterState currentState) throws Exception { .issueDate(issueDate) .type(request.getType()) .expiryDate(expiryDate); - License selfGeneratedLicense = SelfGeneratedLicense.create(specBuilder); + License selfGeneratedLicense = SelfGeneratedLicense.create(specBuilder, currentState.nodes()); LicensesMetaData newLicensesMetaData = new LicensesMetaData(selfGeneratedLicense, Version.CURRENT); mdBuilder.putCustom(LicensesMetaData.TYPE, newLicensesMetaData); return ClusterState.builder(currentState).metaData(mdBuilder).build(); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartupSelfGeneratedLicenseTask.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartupSelfGeneratedLicenseTask.java index 13d6326f3ce1d..c2d53bd071638 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartupSelfGeneratedLicenseTask.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartupSelfGeneratedLicenseTask.java @@ -61,7 +61,7 @@ public ClusterState execute(ClusterState currentState) throws Exception { "]. Must be trial or basic."); } return updateWithLicense(currentState, type); - } else if (LicenseUtils.signatureNeedsUpdate(currentLicensesMetaData.getLicense())) { + } else if (LicenseUtils.signatureNeedsUpdate(currentLicensesMetaData.getLicense(), currentState.nodes())) { return updateLicenseSignature(currentState, currentLicensesMetaData); } else if (LicenseUtils.licenseNeedsExtended(currentLicensesMetaData.getLicense())) { return extendBasic(currentState, currentLicensesMetaData); @@ -87,7 +87,7 @@ private ClusterState updateLicenseSignature(ClusterState currentState, LicensesM .issueDate(issueDate) .type(type) .expiryDate(expiryDate); - License selfGeneratedLicense = SelfGeneratedLicense.create(specBuilder); + License selfGeneratedLicense = SelfGeneratedLicense.create(specBuilder, currentState.nodes()); Version trialVersion = currentLicenseMetaData.getMostRecentTrialVersion(); LicensesMetaData newLicenseMetadata = new LicensesMetaData(selfGeneratedLicense, trialVersion); mdBuilder.putCustom(LicensesMetaData.TYPE, newLicenseMetadata); @@ -120,7 +120,7 @@ private LicensesMetaData createBasicLicenseFromExistingLicense(LicensesMetaData .issueDate(currentLicense.issueDate()) .type("basic") .expiryDate(LicenseService.BASIC_SELF_GENERATED_LICENSE_EXPIRATION_MILLIS); - License selfGeneratedLicense = SelfGeneratedLicense.create(specBuilder); + License selfGeneratedLicense = SelfGeneratedLicense.create(specBuilder, currentLicense.version()); Version trialVersion = currentLicenseMetadata.getMostRecentTrialVersion(); return new LicensesMetaData(selfGeneratedLicense, trialVersion); } @@ -141,7 +141,7 @@ private ClusterState updateWithLicense(ClusterState currentState, String type) { .issueDate(issueDate) .type(type) .expiryDate(expiryDate); - License selfGeneratedLicense = SelfGeneratedLicense.create(specBuilder); + License selfGeneratedLicense = SelfGeneratedLicense.create(specBuilder, currentState.nodes()); LicensesMetaData licensesMetaData; if ("trial".equals(type)) { licensesMetaData = new LicensesMetaData(selfGeneratedLicense, Version.CURRENT); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/license/LicenseRegistrationTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/license/LicenseRegistrationTests.java index 2a237f090e2fd..5405af013af51 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/license/LicenseRegistrationTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/license/LicenseRegistrationTests.java @@ -104,7 +104,7 @@ public void testExpiredSelfGeneratedBasicLicenseIsExtended() throws Exception { .issueDate(dateMath("now-10h", now)) .type("basic") .expiryDate(dateMath("now-2h", now)); - License license = SelfGeneratedLicense.create(builder); + License license = SelfGeneratedLicense.create(builder, License.VERSION_CURRENT); XPackLicenseState licenseState = new XPackLicenseState(Settings.EMPTY); setInitialState(license, licenseState, Settings.EMPTY); @@ -125,4 +125,4 @@ public void testExpiredSelfGeneratedBasicLicenseIsExtended() throws Exception { assertEquals(LicenseService.BASIC_SELF_GENERATED_LICENSE_EXPIRATION_MILLIS, licenseMetaData.getLicense().expiryDate()); assertEquals(uid, licenseMetaData.getLicense().uid()); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/license/LicenseSerializationTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/license/LicenseSerializationTests.java index d7cf5ab50fb48..d07be0fd3c79b 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/license/LicenseSerializationTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/license/LicenseSerializationTests.java @@ -111,7 +111,7 @@ public void testLicenseRestViewNonExpiringBasic() throws Exception { .issueDate(now) .type("basic") .expiryDate(LicenseService.BASIC_SELF_GENERATED_LICENSE_EXPIRATION_MILLIS); - License license = SelfGeneratedLicense.create(specBuilder); + License license = SelfGeneratedLicense.create(specBuilder, License.VERSION_CURRENT); XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON); license.toXContent(builder, new ToXContent.MapParams(Collections.singletonMap(License.REST_VIEW_MODE, "true"))); builder.flush(); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/license/LicensesMetaDataSerializationTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/license/LicensesMetaDataSerializationTests.java index f3ed04ed22dfe..d7799959f6cce 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/license/LicensesMetaDataSerializationTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/license/LicensesMetaDataSerializationTests.java @@ -95,7 +95,7 @@ public void testXContentSerializationOneTrial() throws Exception { .issueDate(issueDate) .type(randomBoolean() ? "trial" : "basic") .expiryDate(issueDate + TimeValue.timeValueHours(2).getMillis()); - final License trialLicense = SelfGeneratedLicense.create(specBuilder); + final License trialLicense = SelfGeneratedLicense.create(specBuilder, License.VERSION_CURRENT); LicensesMetaData licensesMetaData = new LicensesMetaData(trialLicense, Version.CURRENT); XContentBuilder builder = XContentFactory.jsonBuilder(); builder.startObject(); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/license/SelfGeneratedLicenseTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/license/SelfGeneratedLicenseTests.java index aa27dbdcb4964..4e061623ccd94 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/license/SelfGeneratedLicenseTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/license/SelfGeneratedLicenseTests.java @@ -34,7 +34,7 @@ public void testBasic() throws Exception { .type(randomBoolean() ? "trial" : "basic") .issueDate(issueDate) .expiryDate(issueDate + TimeValue.timeValueHours(2).getMillis()); - License trialLicense = SelfGeneratedLicense.create(specBuilder); + License trialLicense = SelfGeneratedLicense.create(specBuilder, License.VERSION_CURRENT); assertThat(SelfGeneratedLicense.verify(trialLicense), equalTo(true)); } @@ -47,7 +47,7 @@ public void testTampered() throws Exception { .maxNodes(5) .issueDate(issueDate) .expiryDate(issueDate + TimeValue.timeValueHours(2).getMillis()); - License trialLicense = SelfGeneratedLicense.create(specBuilder); + License trialLicense = SelfGeneratedLicense.create(specBuilder, License.VERSION_CURRENT); final String originalSignature = trialLicense.signature(); License tamperedLicense = License.builder().fromLicenseSpec(trialLicense, originalSignature) .expiryDate(System.currentTimeMillis() + TimeValue.timeValueHours(5).getMillis()) @@ -70,7 +70,8 @@ public void testFrom1x() throws Exception { .issueDate(issueDate) .expiryDate(issueDate + TimeValue.timeValueHours(2).getMillis()); License pre20TrialLicense = specBuilder.build(); - License license = SelfGeneratedLicense.create(License.builder().fromPre20LicenseSpec(pre20TrialLicense).type("trial")); + License license = SelfGeneratedLicense.create(License.builder().fromPre20LicenseSpec(pre20TrialLicense).type("trial"), + License.VERSION_CURRENT); assertThat(SelfGeneratedLicense.verify(license), equalTo(true)); } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index fb2c58c590307..300cedeb1caf2 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -7,7 +7,6 @@ import org.apache.logging.log4j.Logger; import org.apache.lucene.util.SetOnce; -import org.elasticsearch.ElasticsearchTimeoutException; import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionRequest; @@ -17,7 +16,6 @@ import org.elasticsearch.bootstrap.BootstrapCheck; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.health.ClusterHealthStatus; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.IndexTemplateMetaData; @@ -113,7 +111,6 @@ import org.elasticsearch.xpack.core.security.authc.DefaultAuthenticationFailureHandler; import org.elasticsearch.xpack.core.security.authc.Realm; import org.elasticsearch.xpack.core.security.authc.RealmSettings; -import org.elasticsearch.xpack.core.security.authc.TokenMetaData; import org.elasticsearch.xpack.core.security.authc.esnative.NativeRealmSettings; import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken; import org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField; @@ -1023,7 +1020,8 @@ public BiConsumer getJoinValidator() { if (enabled) { return new ValidateTLSOnJoin(XPackSettings.TRANSPORT_SSL_ENABLED.get(settings), DiscoveryModule.DISCOVERY_TYPE_SETTING.get(settings)) - .andThen(new ValidateUpgradedSecurityIndex()); + .andThen(new ValidateUpgradedSecurityIndex()) + .andThen(new ValidateLicenseCanBeDeserialized()); } return null; } @@ -1060,6 +1058,17 @@ public void accept(DiscoveryNode node, ClusterState state) { } } + static final class ValidateLicenseCanBeDeserialized implements BiConsumer { + @Override + public void accept(DiscoveryNode node, ClusterState state) { + License license = LicenseService.getLicense(state.metaData()); + if (license != null && license.version() >= License.VERSION_CRYPTO_ALGORITHMS && node.getVersion().before(Version.V_6_4_0)) { + throw new IllegalStateException("node " + node + " is on version [" + node.getVersion() + + "] that cannot deserialize the license format [" + license.version() + "], upgrade node to at least 6.4.0"); + } + } + } + @Override public void reloadSPI(ClassLoader loader) { securityExtensions.addAll(SecurityExtension.loadExtensions(loader)); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java index 1cc758c13eaf8..f083bb06d49ad 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java @@ -28,6 +28,7 @@ import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.plugins.MapperPlugin; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.VersionUtils; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.watcher.ResourceWatcherService; import org.elasticsearch.xpack.core.XPackSettings; @@ -278,6 +279,19 @@ public void testTLSJoinValidator() throws Exception { } } + public void testJoinValidatorForLicenseDeserialization() throws Exception { + DiscoveryNode node = new DiscoveryNode("foo", buildNewFakeTransportAddress(), + VersionUtils.randomVersionBetween(random(), null, Version.V_6_3_0)); + MetaData.Builder builder = MetaData.builder(); + License license = TestUtils.generateSignedLicense(null, + randomIntBetween(License.VERSION_CRYPTO_ALGORITHMS, License.VERSION_CURRENT), -1, TimeValue.timeValueHours(24)); + TestUtils.putLicense(builder, license); + ClusterState state = ClusterState.builder(ClusterName.DEFAULT).metaData(builder.build()).build(); + IllegalStateException e = expectThrows(IllegalStateException.class, + () -> new Security.ValidateLicenseCanBeDeserialized().accept(node, state)); + assertThat(e.getMessage(), containsString("cannot deserialize the license format")); + } + public void testIndexJoinValidator_Old_And_Rolling() throws Exception { createComponents(Settings.EMPTY); BiConsumer joinValidator = security.getJoinValidator(); @@ -345,7 +359,7 @@ public void testIndexUpgradeValidatorWithMissingIndex() throws Exception { .nodes(discoveryNodes).build(); joinValidator.accept(node, clusterState); } - + public void testGetFieldFilterSecurityEnabled() throws Exception { createComponents(Settings.EMPTY); Function> fieldFilter = security.getFieldFilter(); From d85960077899ea3f0430e021c7dcd935b63de5c9 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Tue, 5 Jun 2018 12:42:13 +0200 Subject: [PATCH 09/13] Add a doc value format to binary fields. (#30860) This will be necessary for the `docvalue_fields` option to work correctly once we use the field's doc-value format to format doc-value fields. Binary values are formatted as base64-encoded strings. --- .../index/mapper/BinaryFieldMapper.java | 6 +++ .../elasticsearch/search/DocValueFormat.java | 45 +++++++++++++++++++ .../elasticsearch/search/SearchModule.java | 1 + .../search/DocValueFormatTests.java | 14 ++++++ .../search/fields/SearchFieldsIT.java | 14 ++++-- 5 files changed, 77 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/BinaryFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/BinaryFieldMapper.java index 1838b60050e4c..e19bdb6708370 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/BinaryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/BinaryFieldMapper.java @@ -40,6 +40,8 @@ import org.elasticsearch.index.fielddata.plain.BytesBinaryDVIndexFieldData; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.query.QueryShardException; +import org.elasticsearch.search.DocValueFormat; +import org.joda.time.DateTimeZone; import java.io.IOException; import java.util.Base64; @@ -104,6 +106,10 @@ public String typeName() { return CONTENT_TYPE; } + @Override + public DocValueFormat docValueFormat(String format, DateTimeZone timeZone) { + return DocValueFormat.BINARY; + } @Override public BytesReference valueForDisplay(Object value) { diff --git a/server/src/main/java/org/elasticsearch/search/DocValueFormat.java b/server/src/main/java/org/elasticsearch/search/DocValueFormat.java index 8677370fc9927..242e088747341 100644 --- a/server/src/main/java/org/elasticsearch/search/DocValueFormat.java +++ b/server/src/main/java/org/elasticsearch/search/DocValueFormat.java @@ -39,6 +39,7 @@ import java.text.NumberFormat; import java.text.ParseException; import java.util.Arrays; +import java.util.Base64; import java.util.Locale; import java.util.Objects; import java.util.function.LongSupplier; @@ -121,6 +122,50 @@ public BytesRef parseBytesRef(String value) { } }; + DocValueFormat BINARY = new DocValueFormat() { + + @Override + public String getWriteableName() { + return "binary"; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + } + + @Override + public Object format(long value) { + throw new UnsupportedOperationException(); + } + + @Override + public Object format(double value) { + throw new UnsupportedOperationException(); + } + + @Override + public String format(BytesRef value) { + return Base64.getEncoder() + .withoutPadding() + .encodeToString(Arrays.copyOfRange(value.bytes, value.offset, value.offset + value.length)); + } + + @Override + public long parseLong(String value, boolean roundUp, LongSupplier now) { + throw new UnsupportedOperationException(); + } + + @Override + public double parseDouble(String value, boolean roundUp, LongSupplier now) { + throw new UnsupportedOperationException(); + } + + @Override + public BytesRef parseBytesRef(String value) { + return new BytesRef(Base64.getDecoder().decode(value)); + } + }; + final class DateTime implements DocValueFormat { public static final String NAME = "date_time"; diff --git a/server/src/main/java/org/elasticsearch/search/SearchModule.java b/server/src/main/java/org/elasticsearch/search/SearchModule.java index a689c7d1b5505..0faaa14c9205f 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/server/src/main/java/org/elasticsearch/search/SearchModule.java @@ -646,6 +646,7 @@ private void registerValueFormats() { registerValueFormat(DocValueFormat.GEOHASH.getWriteableName(), in -> DocValueFormat.GEOHASH); registerValueFormat(DocValueFormat.IP.getWriteableName(), in -> DocValueFormat.IP); registerValueFormat(DocValueFormat.RAW.getWriteableName(), in -> DocValueFormat.RAW); + registerValueFormat(DocValueFormat.BINARY.getWriteableName(), in -> DocValueFormat.BINARY); } /** diff --git a/server/src/test/java/org/elasticsearch/search/DocValueFormatTests.java b/server/src/test/java/org/elasticsearch/search/DocValueFormatTests.java index e5cfbf98b3db9..0190627947448 100644 --- a/server/src/test/java/org/elasticsearch/search/DocValueFormatTests.java +++ b/server/src/test/java/org/elasticsearch/search/DocValueFormatTests.java @@ -44,6 +44,7 @@ public void testSerialization() throws Exception { entries.add(new Entry(DocValueFormat.class, DocValueFormat.GEOHASH.getWriteableName(), in -> DocValueFormat.GEOHASH)); entries.add(new Entry(DocValueFormat.class, DocValueFormat.IP.getWriteableName(), in -> DocValueFormat.IP)); entries.add(new Entry(DocValueFormat.class, DocValueFormat.RAW.getWriteableName(), in -> DocValueFormat.RAW)); + entries.add(new Entry(DocValueFormat.class, DocValueFormat.BINARY.getWriteableName(), in -> DocValueFormat.BINARY)); NamedWriteableRegistry registry = new NamedWriteableRegistry(entries); BytesStreamOutput out = new BytesStreamOutput(); @@ -82,6 +83,11 @@ public void testSerialization() throws Exception { out.writeNamedWriteable(DocValueFormat.RAW); in = new NamedWriteableAwareStreamInput(out.bytes().streamInput(), registry); assertSame(DocValueFormat.RAW, in.readNamedWriteable(DocValueFormat.class)); + + out = new BytesStreamOutput(); + out.writeNamedWriteable(DocValueFormat.BINARY); + in = new NamedWriteableAwareStreamInput(out.bytes().streamInput(), registry); + assertSame(DocValueFormat.BINARY, in.readNamedWriteable(DocValueFormat.class)); } public void testRawFormat() { @@ -96,6 +102,14 @@ public void testRawFormat() { assertEquals("abc", DocValueFormat.RAW.format(new BytesRef("abc"))); } + public void testBinaryFormat() { + assertEquals("", DocValueFormat.BINARY.format(new BytesRef())); + assertEquals("KmQ", DocValueFormat.BINARY.format(new BytesRef(new byte[] {42, 100}))); + + assertEquals(new BytesRef(), DocValueFormat.BINARY.parseBytesRef("")); + assertEquals(new BytesRef(new byte[] {42, 100}), DocValueFormat.BINARY.parseBytesRef("KmQ")); + } + public void testBooleanFormat() { assertEquals(false, DocValueFormat.BOOLEAN.format(0)); assertEquals(true, DocValueFormat.BOOLEAN.format(1)); diff --git a/server/src/test/java/org/elasticsearch/search/fields/SearchFieldsIT.java b/server/src/test/java/org/elasticsearch/search/fields/SearchFieldsIT.java index 2fcd03c1e04ee..ce444be6772eb 100644 --- a/server/src/test/java/org/elasticsearch/search/fields/SearchFieldsIT.java +++ b/server/src/test/java/org/elasticsearch/search/fields/SearchFieldsIT.java @@ -19,6 +19,7 @@ package org.elasticsearch.search.fields; +import org.apache.lucene.util.BytesRef; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchRequestBuilder; import org.elasticsearch.action.search.SearchResponse; @@ -719,7 +720,7 @@ public void testSingleValueFieldDatatField() throws ExecutionException, Interrup assertThat(fields.get("test_field").getValue(), equalTo("foobar")); } - public void testFieldsPulledFromFieldData() throws Exception { + public void testDocValueFields() throws Exception { createIndex("test"); String mapping = Strings @@ -763,6 +764,7 @@ public void testFieldsPulledFromFieldData() throws Exception { .endObject() .startObject("binary_field") .field("type", "binary") + .field("doc_values", true) // off by default on binary fields .endObject() .startObject("ip_field") .field("type", "ip") @@ -785,6 +787,7 @@ public void testFieldsPulledFromFieldData() throws Exception { .field("double_field", 6.0d) .field("date_field", Joda.forPattern("dateOptionalTime").printer().print(date)) .field("boolean_field", true) + .field("binary_field", new byte[] {42, 100}) .field("ip_field", "::1") .endObject()).execute().actionGet(); @@ -801,6 +804,7 @@ public void testFieldsPulledFromFieldData() throws Exception { .addDocValueField("double_field") .addDocValueField("date_field") .addDocValueField("boolean_field") + .addDocValueField("binary_field") .addDocValueField("ip_field"); SearchResponse searchResponse = builder.execute().actionGet(); @@ -809,7 +813,7 @@ public void testFieldsPulledFromFieldData() throws Exception { Set fields = new HashSet<>(searchResponse.getHits().getAt(0).getFields().keySet()); assertThat(fields, equalTo(newHashSet("byte_field", "short_field", "integer_field", "long_field", "float_field", "double_field", "date_field", "boolean_field", "text_field", "keyword_field", - "ip_field"))); + "binary_field", "ip_field"))); assertThat(searchResponse.getHits().getAt(0).getFields().get("byte_field").getValue().toString(), equalTo("1")); assertThat(searchResponse.getHits().getAt(0).getFields().get("short_field").getValue().toString(), equalTo("2")); @@ -821,6 +825,8 @@ public void testFieldsPulledFromFieldData() throws Exception { assertThat(searchResponse.getHits().getAt(0).getFields().get("boolean_field").getValue(), equalTo((Object) true)); assertThat(searchResponse.getHits().getAt(0).getFields().get("text_field").getValue(), equalTo("foo")); assertThat(searchResponse.getHits().getAt(0).getFields().get("keyword_field").getValue(), equalTo("foo")); + assertThat(searchResponse.getHits().getAt(0).getFields().get("binary_field").getValue(), + equalTo(new BytesRef(new byte[] {42, 100}))); assertThat(searchResponse.getHits().getAt(0).getFields().get("ip_field").getValue(), equalTo("::1")); builder = client().prepareSearch().setQuery(matchAllQuery()) @@ -834,6 +840,7 @@ public void testFieldsPulledFromFieldData() throws Exception { .addDocValueField("double_field", "use_field_mapping") .addDocValueField("date_field", "use_field_mapping") .addDocValueField("boolean_field", "use_field_mapping") + .addDocValueField("binary_field", "use_field_mapping") .addDocValueField("ip_field", "use_field_mapping"); searchResponse = builder.execute().actionGet(); @@ -842,7 +849,7 @@ public void testFieldsPulledFromFieldData() throws Exception { fields = new HashSet<>(searchResponse.getHits().getAt(0).getFields().keySet()); assertThat(fields, equalTo(newHashSet("byte_field", "short_field", "integer_field", "long_field", "float_field", "double_field", "date_field", "boolean_field", "text_field", "keyword_field", - "ip_field"))); + "binary_field", "ip_field"))); assertThat(searchResponse.getHits().getAt(0).getFields().get("byte_field").getValue().toString(), equalTo("1")); assertThat(searchResponse.getHits().getAt(0).getFields().get("short_field").getValue().toString(), equalTo("2")); @@ -855,6 +862,7 @@ public void testFieldsPulledFromFieldData() throws Exception { assertThat(searchResponse.getHits().getAt(0).getFields().get("boolean_field").getValue(), equalTo((Object) true)); assertThat(searchResponse.getHits().getAt(0).getFields().get("text_field").getValue(), equalTo("foo")); assertThat(searchResponse.getHits().getAt(0).getFields().get("keyword_field").getValue(), equalTo("foo")); + assertThat(searchResponse.getHits().getAt(0).getFields().get("binary_field").getValue(), equalTo("KmQ")); assertThat(searchResponse.getHits().getAt(0).getFields().get("ip_field").getValue(), equalTo("::1")); builder = client().prepareSearch().setQuery(matchAllQuery()) From 916d15d06f723ada6b62405c0528ba5d5e3a5543 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Tue, 5 Jun 2018 14:54:24 +0200 Subject: [PATCH 10/13] Fix docs build. --- docs/reference/query-dsl/query-string-syntax.asciidoc | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/reference/query-dsl/query-string-syntax.asciidoc b/docs/reference/query-dsl/query-string-syntax.asciidoc index ca852ee728b5d..38045539d7e7d 100644 --- a/docs/reference/query-dsl/query-string-syntax.asciidoc +++ b/docs/reference/query-dsl/query-string-syntax.asciidoc @@ -256,7 +256,6 @@ would look like this: } } -**** ===== Grouping From 674253017fa2d6e030ab7429b9dae097410db4cb Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Tue, 5 Jun 2018 15:26:21 +0200 Subject: [PATCH 11/13] Fix rest test skip version multi_span prefix queries do not work with the prefix field before 6.4. Relates #31066 --- .../rest-api-spec/test/search/190_index_prefix_search.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/190_index_prefix_search.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/190_index_prefix_search.yml index 92b607704a712..fec8becb8fade 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/190_index_prefix_search.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/190_index_prefix_search.yml @@ -66,7 +66,7 @@ setup: --- "search index prefixes with span_multi": - skip: - version: " - 6.2.99" + version: " - 6.3.99" reason: span_multi throws an exception with prefix fields on < versions - do: From e7965ae5c438e485edf895fc9781ba8ebdd88393 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 5 Jun 2018 15:29:00 +0200 Subject: [PATCH 12/13] [Tests] Muting RatedRequestsTests#testXContentParsingIsNotLenient --- .../org/elasticsearch/index/rankeval/RatedRequestsTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedRequestsTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedRequestsTests.java index 084f29b8c9a87..1be1acb1317dc 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedRequestsTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedRequestsTests.java @@ -131,6 +131,7 @@ public void testXContentRoundtrip() throws IOException { } } + @AwaitsFix(bugUrl="https://github.com/elastic/elasticsearch/issues/31104") public void testXContentParsingIsNotLenient() throws IOException { RatedRequest testItem = createTestItem(randomBoolean()); XContentType xContentType = randomFrom(XContentType.values()); From f485e8cbb1ef4c1f50c61d6d580bdf9ad1c134ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 5 Jun 2018 15:49:16 +0200 Subject: [PATCH 13/13] Share common readFrom/writeTo code in AcknowledgeResponse (#30983) The majority of Responses inheriting from AcknowledgeResponse implement the readFrom and writeTo serialization method in the same way. Moving this as a default into AcknowledgeResponse and letting the few exceptions that need a slightly different implementation handle this themselves saves a lot of duplication. --- .../delete/DeleteRepositoryResponse.java | 17 ----- .../put/PutRepositoryResponse.java | 16 ----- .../reroute/ClusterRerouteResponse.java | 28 +++++--- .../ClusterUpdateSettingsResponse.java | 27 ++++--- .../delete/DeleteSnapshotResponse.java | 17 ----- .../DeleteStoredScriptResponse.java | 16 ----- .../PutStoredScriptResponse.java | 16 ----- .../indices/alias/IndicesAliasesResponse.java | 16 ----- .../indices/close/CloseIndexResponse.java | 16 ----- .../indices/create/CreateIndexResponse.java | 2 - .../indices/delete/DeleteIndexResponse.java | 17 ----- .../mapping/put/PutMappingResponse.java | 16 ----- .../admin/indices/open/OpenIndexResponse.java | 4 -- .../indices/rollover/RolloverResponse.java | 71 +++++++++++++------ .../settings/put/UpdateSettingsResponse.java | 16 ----- .../delete/DeleteIndexTemplateResponse.java | 16 ----- .../put/PutIndexTemplateResponse.java | 16 ----- .../upgrade/post/UpgradeSettingsResponse.java | 16 ----- .../action/ingest/WritePipelineResponse.java | 16 ----- .../support/master/AcknowledgedResponse.java | 16 ++--- .../ClusterUpdateSettingsResponseTests.java | 8 +++ .../rollover/RolloverResponseTests.java | 8 +++ .../license/DeleteLicenseResponse.java | 17 ----- .../license/PostStartBasicResponse.java | 2 - .../license/PutLicenseResponse.java | 2 - .../core/ml/action/DeleteCalendarAction.java | 12 ---- .../ml/action/DeleteCalendarEventAction.java | 12 ---- .../core/ml/action/DeleteDatafeedAction.java | 12 ---- .../core/ml/action/DeleteFilterAction.java | 12 ---- .../xpack/core/ml/action/DeleteJobAction.java | 12 ---- .../ml/action/DeleteModelSnapshotAction.java | 13 ---- .../ml/action/FinalizeJobExecutionAction.java | 12 ---- .../xpack/core/ml/action/OpenJobAction.java | 11 +-- .../xpack/core/ml/action/PutFilterAction.java | 13 ---- .../core/ml/action/StartDatafeedAction.java | 10 --- .../ml/action/ValidateDetectorAction.java | 12 ---- .../ml/action/ValidateJobConfigAction.java | 12 ---- .../rollup/action/DeleteRollupJobAction.java | 12 ---- .../rollup/action/PutRollupJobAction.java | 12 ---- .../service/WatcherServiceResponse.java | 16 ----- 40 files changed, 111 insertions(+), 484 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteRepositoryResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteRepositoryResponse.java index 43036a2a697ef..1f1fe524c64a9 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteRepositoryResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteRepositoryResponse.java @@ -20,14 +20,9 @@ package org.elasticsearch.action.admin.cluster.repositories.delete; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ConstructingObjectParser; -import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentParser; -import java.io.IOException; - /** * Unregister repository response */ @@ -47,18 +42,6 @@ public class DeleteRepositoryResponse extends AcknowledgedResponse { super(acknowledged); } - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } - public static DeleteRepositoryResponse fromXContent(XContentParser parser) { return PARSER.apply(parser, null); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutRepositoryResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutRepositoryResponse.java index e58a1d9d147f9..52a1a736ec7c1 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutRepositoryResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutRepositoryResponse.java @@ -20,13 +20,9 @@ package org.elasticsearch.action.admin.cluster.repositories.put; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.XContentParser; -import java.io.IOException; - /** * Register repository response */ @@ -46,18 +42,6 @@ public class PutRepositoryResponse extends AcknowledgedResponse { super(acknowledged); } - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } - public static PutRepositoryResponse fromXContent(XContentParser parser) { return PARSER.apply(parser, null); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteResponse.java index caf9fa62d4cd1..3c35b97702429 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteResponse.java @@ -63,22 +63,32 @@ public RoutingExplanations getExplanations() { @Override public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - state = ClusterState.readFrom(in, null); - readAcknowledged(in); - explanations = RoutingExplanations.readFrom(in); + if (in.getVersion().onOrAfter(Version.V_6_4_0)) { + super.readFrom(in); + state = ClusterState.readFrom(in, null); + explanations = RoutingExplanations.readFrom(in); + } else { + state = ClusterState.readFrom(in, null); + acknowledged = in.readBoolean(); + explanations = RoutingExplanations.readFrom(in); + } } @Override public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - if (out.getVersion().onOrAfter(Version.V_6_3_0)) { + if (out.getVersion().onOrAfter(Version.V_6_4_0)) { + super.writeTo(out); state.writeTo(out); + RoutingExplanations.writeTo(explanations, out); } else { - ClusterModule.filterCustomsForPre63Clients(state).writeTo(out); + if (out.getVersion().onOrAfter(Version.V_6_3_0)) { + state.writeTo(out); + } else { + ClusterModule.filterCustomsForPre63Clients(state).writeTo(out); + } + out.writeBoolean(acknowledged); + RoutingExplanations.writeTo(explanations, out); } - writeAcknowledged(out); - RoutingExplanations.writeTo(explanations, out); } @Override diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsResponse.java index 9ce22268afd8d..cc29e60aa996f 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsResponse.java @@ -19,6 +19,7 @@ package org.elasticsearch.action.admin.cluster.settings; +import org.elasticsearch.Version; import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.io.stream.StreamInput; @@ -67,10 +68,15 @@ public class ClusterUpdateSettingsResponse extends AcknowledgedResponse { @Override public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - transientSettings = Settings.readSettingsFromStream(in); - persistentSettings = Settings.readSettingsFromStream(in); - readAcknowledged(in); + if (in.getVersion().onOrAfter(Version.V_6_4_0)) { + super.readFrom(in); + transientSettings = Settings.readSettingsFromStream(in); + persistentSettings = Settings.readSettingsFromStream(in); + } else { + transientSettings = Settings.readSettingsFromStream(in); + persistentSettings = Settings.readSettingsFromStream(in); + acknowledged = in.readBoolean(); + } } public Settings getTransientSettings() { @@ -83,10 +89,15 @@ public Settings getPersistentSettings() { @Override public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - Settings.writeSettingsToStream(transientSettings, out); - Settings.writeSettingsToStream(persistentSettings, out); - writeAcknowledged(out); + if (out.getVersion().onOrAfter(Version.V_6_4_0)) { + super.writeTo(out); + Settings.writeSettingsToStream(transientSettings, out); + Settings.writeSettingsToStream(persistentSettings, out); + } else { + Settings.writeSettingsToStream(transientSettings, out); + Settings.writeSettingsToStream(persistentSettings, out); + out.writeBoolean(acknowledged); + } } @Override diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/delete/DeleteSnapshotResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/delete/DeleteSnapshotResponse.java index 0783f3061efba..d8de78c3e5b76 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/delete/DeleteSnapshotResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/delete/DeleteSnapshotResponse.java @@ -20,10 +20,6 @@ package org.elasticsearch.action.admin.cluster.snapshots.delete; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; - -import java.io.IOException; /** * Delete snapshot response @@ -36,17 +32,4 @@ public class DeleteSnapshotResponse extends AcknowledgedResponse { DeleteSnapshotResponse(boolean acknowledged) { super(acknowledged); } - - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } - } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/DeleteStoredScriptResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/DeleteStoredScriptResponse.java index 4fbb50a34d978..42f08ae73e06d 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/DeleteStoredScriptResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/DeleteStoredScriptResponse.java @@ -20,10 +20,6 @@ package org.elasticsearch.action.admin.cluster.storedscripts; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; - -import java.io.IOException; public class DeleteStoredScriptResponse extends AcknowledgedResponse { @@ -33,16 +29,4 @@ public class DeleteStoredScriptResponse extends AcknowledgedResponse { public DeleteStoredScriptResponse(boolean acknowledged) { super(acknowledged); } - - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/PutStoredScriptResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/PutStoredScriptResponse.java index 44250f3809d96..a511c7dd47e99 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/PutStoredScriptResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/storedscripts/PutStoredScriptResponse.java @@ -20,10 +20,6 @@ package org.elasticsearch.action.admin.cluster.storedscripts; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; - -import java.io.IOException; public class PutStoredScriptResponse extends AcknowledgedResponse { @@ -34,16 +30,4 @@ public PutStoredScriptResponse(boolean acknowledged) { super(acknowledged); } - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } - } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java index b6402d5139aad..ebfc82fec74ca 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesResponse.java @@ -20,13 +20,9 @@ package org.elasticsearch.action.admin.indices.alias; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.XContentParser; -import java.io.IOException; - /** * A response for a add/remove alias action. */ @@ -45,18 +41,6 @@ public class IndicesAliasesResponse extends AcknowledgedResponse { super(acknowledged); } - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } - public static IndicesAliasesResponse fromXContent(XContentParser parser) { return PARSER.apply(parser, null); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/close/CloseIndexResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/close/CloseIndexResponse.java index de56c52f9f6de..bfebaee5e59da 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/close/CloseIndexResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/close/CloseIndexResponse.java @@ -20,13 +20,9 @@ package org.elasticsearch.action.admin.indices.close; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.XContentParser; -import java.io.IOException; - /** * A response for a close index action. */ @@ -45,18 +41,6 @@ public class CloseIndexResponse extends AcknowledgedResponse { super(acknowledged); } - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } - public static CloseIndexResponse fromXContent(XContentParser parser) { return PARSER.apply(parser, null); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexResponse.java index 13a9a948ca80d..8e01142dfafd3 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexResponse.java @@ -66,7 +66,6 @@ protected CreateIndexResponse(boolean acknowledged, boolean shardsAcknowledged, @Override public void readFrom(StreamInput in) throws IOException { super.readFrom(in); - readAcknowledged(in); readShardsAcknowledged(in); if (in.getVersion().onOrAfter(Version.V_5_6_0)) { index = in.readString(); @@ -76,7 +75,6 @@ public void readFrom(StreamInput in) throws IOException { @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); - writeAcknowledged(out); writeShardsAcknowledged(out); if (out.getVersion().onOrAfter(Version.V_5_6_0)) { out.writeString(index); diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/delete/DeleteIndexResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/delete/DeleteIndexResponse.java index a788f272d1877..3a04dc5b70be3 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/delete/DeleteIndexResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/delete/DeleteIndexResponse.java @@ -20,14 +20,9 @@ package org.elasticsearch.action.admin.indices.delete; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ConstructingObjectParser; -import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; -import java.io.IOException; - /** * A response for a delete index action. */ @@ -47,18 +42,6 @@ public class DeleteIndexResponse extends AcknowledgedResponse { super(acknowledged); } - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } - public static DeleteIndexResponse fromXContent(XContentParser parser) { return PARSER.apply(parser, null); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingResponse.java index 8ccc5c8006a18..1e022474955a8 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingResponse.java @@ -20,13 +20,9 @@ package org.elasticsearch.action.admin.indices.mapping.put; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.XContentParser; -import java.io.IOException; - /** * The response of put mapping operation. */ @@ -47,18 +43,6 @@ protected PutMappingResponse(boolean acknowledged) { super(acknowledged); } - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } - public static PutMappingResponse fromXContent(XContentParser parser) { return PARSER.apply(parser, null); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/open/OpenIndexResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/open/OpenIndexResponse.java index 3918273cec90d..97db91f8973f4 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/open/OpenIndexResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/open/OpenIndexResponse.java @@ -21,11 +21,9 @@ import org.elasticsearch.Version; import org.elasticsearch.action.support.master.ShardsAcknowledgedResponse; -import org.elasticsearch.common.ParseField; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ConstructingObjectParser; -import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import java.io.IOException; @@ -52,7 +50,6 @@ public class OpenIndexResponse extends ShardsAcknowledgedResponse { @Override public void readFrom(StreamInput in) throws IOException { super.readFrom(in); - readAcknowledged(in); if (in.getVersion().onOrAfter(Version.V_6_1_0)) { readShardsAcknowledged(in); } @@ -61,7 +58,6 @@ public void readFrom(StreamInput in) throws IOException { @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); - writeAcknowledged(out); if (out.getVersion().onOrAfter(Version.V_6_1_0)) { writeShardsAcknowledged(out); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverResponse.java index 3e5051a4dcb48..b7e4294a5635c 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverResponse.java @@ -19,6 +19,7 @@ package org.elasticsearch.action.admin.indices.rollover; +import org.elasticsearch.Version; import org.elasticsearch.action.support.master.ShardsAcknowledgedResponse; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.io.stream.StreamInput; @@ -126,34 +127,60 @@ public boolean isShardsAcked() { @Override public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - oldIndex = in.readString(); - newIndex = in.readString(); - int conditionSize = in.readVInt(); - conditionStatus = new HashMap<>(conditionSize); - for (int i = 0; i < conditionSize; i++) { - conditionStatus.put(in.readString(), in.readBoolean()); + if (in.getVersion().onOrAfter(Version.V_6_4_0)) { + super.readFrom(in); + oldIndex = in.readString(); + newIndex = in.readString(); + int conditionSize = in.readVInt(); + conditionStatus = new HashMap<>(conditionSize); + for (int i = 0; i < conditionSize; i++) { + conditionStatus.put(in.readString(), in.readBoolean()); + } + dryRun = in.readBoolean(); + rolledOver = in.readBoolean(); + readShardsAcknowledged(in); + } else { + oldIndex = in.readString(); + newIndex = in.readString(); + int conditionSize = in.readVInt(); + conditionStatus = new HashMap<>(conditionSize); + for (int i = 0; i < conditionSize; i++) { + conditionStatus.put(in.readString(), in.readBoolean()); + } + dryRun = in.readBoolean(); + rolledOver = in.readBoolean(); + acknowledged = in.readBoolean(); + readShardsAcknowledged(in); } - dryRun = in.readBoolean(); - rolledOver = in.readBoolean(); - readAcknowledged(in); - readShardsAcknowledged(in); } @Override public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - out.writeString(oldIndex); - out.writeString(newIndex); - out.writeVInt(conditionStatus.size()); - for (Map.Entry entry : conditionStatus.entrySet()) { - out.writeString(entry.getKey()); - out.writeBoolean(entry.getValue()); + if (out.getVersion().onOrAfter(Version.V_6_4_0)) { + super.writeTo(out); + out.writeString(oldIndex); + out.writeString(newIndex); + out.writeVInt(conditionStatus.size()); + for (Map.Entry entry : conditionStatus.entrySet()) { + out.writeString(entry.getKey()); + out.writeBoolean(entry.getValue()); + } + out.writeBoolean(dryRun); + out.writeBoolean(rolledOver); + writeShardsAcknowledged(out); + } else { + out.writeString(oldIndex); + out.writeString(newIndex); + out.writeVInt(conditionStatus.size()); + for (Map.Entry entry : conditionStatus.entrySet()) { + out.writeString(entry.getKey()); + out.writeBoolean(entry.getValue()); + } + out.writeBoolean(dryRun); + out.writeBoolean(rolledOver); + out.writeBoolean(acknowledged); + writeShardsAcknowledged(out); } - out.writeBoolean(dryRun); - out.writeBoolean(rolledOver); - writeAcknowledged(out); - writeShardsAcknowledged(out); } @Override diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/UpdateSettingsResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/UpdateSettingsResponse.java index 79116eb8cf5a7..022f575f1d0e5 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/UpdateSettingsResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/UpdateSettingsResponse.java @@ -20,13 +20,9 @@ package org.elasticsearch.action.admin.indices.settings.put; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.XContentParser; -import java.io.IOException; - /** * A response for an update index settings action */ @@ -46,18 +42,6 @@ public class UpdateSettingsResponse extends AcknowledgedResponse { super(acknowledged); } - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } - public static UpdateSettingsResponse fromXContent(XContentParser parser) { return PARSER.apply(parser, null); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/template/delete/DeleteIndexTemplateResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/template/delete/DeleteIndexTemplateResponse.java index 9519f0f9fcf4f..a95f18bf13de0 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/template/delete/DeleteIndexTemplateResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/template/delete/DeleteIndexTemplateResponse.java @@ -19,10 +19,6 @@ package org.elasticsearch.action.admin.indices.template.delete; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; - -import java.io.IOException; /** * A response for a delete index template. @@ -35,16 +31,4 @@ public class DeleteIndexTemplateResponse extends AcknowledgedResponse { protected DeleteIndexTemplateResponse(boolean acknowledged) { super(acknowledged); } - - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateResponse.java index 6c8a5291b12d5..59b00bd719b33 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/template/put/PutIndexTemplateResponse.java @@ -19,13 +19,9 @@ package org.elasticsearch.action.admin.indices.template.put; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.XContentParser; -import java.io.IOException; - /** * A response for a put index template action. */ @@ -38,18 +34,6 @@ protected PutIndexTemplateResponse(boolean acknowledged) { super(acknowledged); } - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } - private static final ConstructingObjectParser PARSER; static { PARSER = new ConstructingObjectParser<>("put_index_template", true, args -> new PutIndexTemplateResponse((boolean) args[0])); diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/upgrade/post/UpgradeSettingsResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/upgrade/post/UpgradeSettingsResponse.java index 0918af6f41807..0d3ac16a6fe14 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/upgrade/post/UpgradeSettingsResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/upgrade/post/UpgradeSettingsResponse.java @@ -20,10 +20,6 @@ package org.elasticsearch.action.admin.indices.upgrade.post; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; - -import java.io.IOException; /** * A response for an update index settings action @@ -36,16 +32,4 @@ public class UpgradeSettingsResponse extends AcknowledgedResponse { UpgradeSettingsResponse(boolean acknowledged) { super(acknowledged); } - - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } } diff --git a/server/src/main/java/org/elasticsearch/action/ingest/WritePipelineResponse.java b/server/src/main/java/org/elasticsearch/action/ingest/WritePipelineResponse.java index 36301d6735a02..b3bda3f152196 100644 --- a/server/src/main/java/org/elasticsearch/action/ingest/WritePipelineResponse.java +++ b/server/src/main/java/org/elasticsearch/action/ingest/WritePipelineResponse.java @@ -20,14 +20,10 @@ package org.elasticsearch.action.ingest; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentParser; -import java.io.IOException; - public class WritePipelineResponse extends AcknowledgedResponse implements ToXContentObject { private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( @@ -45,18 +41,6 @@ public WritePipelineResponse(boolean acknowledged) { super(acknowledged); } - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } - public static WritePipelineResponse fromXContent(XContentParser parser) { return PARSER.apply(parser, null); } diff --git a/server/src/main/java/org/elasticsearch/action/support/master/AcknowledgedResponse.java b/server/src/main/java/org/elasticsearch/action/support/master/AcknowledgedResponse.java index ea9a373efdd76..41c806bc20586 100644 --- a/server/src/main/java/org/elasticsearch/action/support/master/AcknowledgedResponse.java +++ b/server/src/main/java/org/elasticsearch/action/support/master/AcknowledgedResponse.java @@ -45,7 +45,7 @@ protected static void declareAcknowledgedField( ObjectParser.ValueType.BOOLEAN); } - private boolean acknowledged; + protected boolean acknowledged; protected AcknowledgedResponse() { @@ -63,17 +63,15 @@ public final boolean isAcknowledged() { return acknowledged; } - /** - * Reads the acknowledged value - */ - protected void readAcknowledged(StreamInput in) throws IOException { + @Override + public void readFrom(StreamInput in) throws IOException { + super.readFrom(in); acknowledged = in.readBoolean(); } - /** - * Writes the acknowledged value - */ - protected void writeAcknowledged(StreamOutput out) throws IOException { + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); out.writeBoolean(acknowledged); } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsResponseTests.java index 5ea5fd5ac0226..e8bd14b640dfa 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsResponseTests.java @@ -19,13 +19,16 @@ package org.elasticsearch.action.admin.cluster.settings; +import org.elasticsearch.Version; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.Settings.Builder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.test.AbstractStreamableXContentTestCase; +import org.elasticsearch.test.VersionUtils; +import java.io.IOException; import java.util.List; import java.util.Set; import java.util.function.Predicate; @@ -96,4 +99,9 @@ protected ClusterUpdateSettingsResponse createTestInstance() { protected ClusterUpdateSettingsResponse createBlankInstance() { return new ClusterUpdateSettingsResponse(); } + + public void testOldSerialisation() throws IOException { + ClusterUpdateSettingsResponse original = createTestInstance(); + assertSerialization(original, VersionUtils.randomVersionBetween(random(), Version.V_6_0_0, Version.V_6_4_0)); + } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverResponseTests.java index a50f06cc54038..e0f58ecd92daf 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverResponseTests.java @@ -19,10 +19,13 @@ package org.elasticsearch.action.admin.indices.rollover; +import org.elasticsearch.Version; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.test.AbstractStreamableXContentTestCase; +import org.elasticsearch.test.VersionUtils; +import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -126,4 +129,9 @@ protected RolloverResponse mutateInstance(RolloverResponse response) { throw new UnsupportedOperationException(); } } + + public void testOldSerialisation() throws IOException { + RolloverResponse original = createTestInstance(); + assertSerialization(original, VersionUtils.randomVersionBetween(random(), Version.V_6_0_0, Version.V_6_4_0)); + } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/DeleteLicenseResponse.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/DeleteLicenseResponse.java index c30890a0ff6bf..0dd092d6fe6ae 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/DeleteLicenseResponse.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/DeleteLicenseResponse.java @@ -6,10 +6,6 @@ package org.elasticsearch.license; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; - -import java.io.IOException; public class DeleteLicenseResponse extends AcknowledgedResponse { @@ -19,17 +15,4 @@ public class DeleteLicenseResponse extends AcknowledgedResponse { DeleteLicenseResponse(boolean acknowledged) { super(acknowledged); } - - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } - } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/PostStartBasicResponse.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/PostStartBasicResponse.java index 985c3689e6d7c..66fffbe28d397 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/PostStartBasicResponse.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/PostStartBasicResponse.java @@ -74,7 +74,6 @@ public Status getStatus() { @Override public void readFrom(StreamInput in) throws IOException { super.readFrom(in); - readAcknowledged(in); status = in.readEnum(Status.class); acknowledgeMessage = in.readOptionalString(); int size = in.readVInt(); @@ -94,7 +93,6 @@ public void readFrom(StreamInput in) throws IOException { @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); - writeAcknowledged(out); out.writeEnum(status); out.writeOptionalString(acknowledgeMessage); out.writeVInt(acknowledgeMessages.size()); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/PutLicenseResponse.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/PutLicenseResponse.java index a17836a836b66..c85bb068da39e 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/PutLicenseResponse.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/PutLicenseResponse.java @@ -52,7 +52,6 @@ public String acknowledgeHeader() { @Override public void readFrom(StreamInput in) throws IOException { super.readFrom(in); - readAcknowledged(in); status = LicensesStatus.fromId(in.readVInt()); acknowledgeHeader = in.readOptionalString(); int size = in.readVInt(); @@ -72,7 +71,6 @@ public void readFrom(StreamInput in) throws IOException { @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); - writeAcknowledged(out); out.writeVInt(status.id()); out.writeOptionalString(acknowledgeHeader); out.writeVInt(acknowledgeMessages.size()); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteCalendarAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteCalendarAction.java index 2a01282d41115..a680855d4fd78 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteCalendarAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteCalendarAction.java @@ -103,17 +103,5 @@ public Response(boolean acknowledged) { } public Response() {} - - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteCalendarEventAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteCalendarEventAction.java index 01d5e3e37215b..572aaec85766b 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteCalendarEventAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteCalendarEventAction.java @@ -109,17 +109,5 @@ public Response(boolean acknowledged) { } private Response() {} - - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteDatafeedAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteDatafeedAction.java index 6876294bdf301..f6e06c69bc178 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteDatafeedAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteDatafeedAction.java @@ -127,18 +127,6 @@ public Response() { public Response(boolean acknowledged) { super(acknowledged); } - - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteFilterAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteFilterAction.java index 86e343fda22cc..076b65c175ed1 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteFilterAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteFilterAction.java @@ -103,18 +103,6 @@ public Response(boolean acknowledged) { } public Response() {} - - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteJobAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteJobAction.java index 4fa264862dbaf..fbaf9850ab329 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteJobAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteJobAction.java @@ -130,18 +130,6 @@ public Response(boolean acknowledged) { } public Response() {} - - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } } } \ No newline at end of file diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteModelSnapshotAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteModelSnapshotAction.java index 39c497ab51ea6..9085b9af8f7d6 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteModelSnapshotAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/DeleteModelSnapshotAction.java @@ -87,19 +87,6 @@ public Response(boolean acknowledged) { } public Response() {} - - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } - } public static class RequestBuilder extends ActionRequestBuilder { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/FinalizeJobExecutionAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/FinalizeJobExecutionAction.java index 9259aa0c60473..c4b369e0cf574 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/FinalizeJobExecutionAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/FinalizeJobExecutionAction.java @@ -85,18 +85,6 @@ public Response(boolean acknowledged) { public Response() { } - - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/OpenJobAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/OpenJobAction.java index eb102cdc3a68a..a8f3d08061dd1 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/OpenJobAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/OpenJobAction.java @@ -21,6 +21,7 @@ import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.persistent.PersistentTaskParams; import org.elasticsearch.tasks.Task; import org.elasticsearch.xpack.core.XPackPlugin; import org.elasticsearch.xpack.core.ml.MachineLearningField; @@ -257,16 +258,6 @@ public Response(boolean acknowledged) { super(acknowledged); } - @Override - public void readFrom(StreamInput in) throws IOException { - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - writeAcknowledged(out); - } - @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/PutFilterAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/PutFilterAction.java index a5d58d8576c8b..38e4cf9cadb8e 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/PutFilterAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/PutFilterAction.java @@ -125,18 +125,5 @@ public static class Response extends AcknowledgedResponse { public Response() { super(true); } - - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } - } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/StartDatafeedAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/StartDatafeedAction.java index df23fb00c89f3..35acc23a7b805 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/StartDatafeedAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/StartDatafeedAction.java @@ -294,16 +294,6 @@ public Response(boolean acknowledged) { super(acknowledged); } - @Override - public void readFrom(StreamInput in) throws IOException { - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - writeAcknowledged(out); - } - @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateDetectorAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateDetectorAction.java index 13948fc5fdc8f..5254f4aebf25f 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateDetectorAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateDetectorAction.java @@ -121,18 +121,6 @@ public Response() { public Response(boolean acknowledged) { super(acknowledged); } - - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateJobConfigAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateJobConfigAction.java index e0cde4f9358c9..ee5dfb27688d2 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateJobConfigAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ValidateJobConfigAction.java @@ -127,18 +127,6 @@ public Response() { public Response(boolean acknowledged) { super(acknowledged); } - - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/action/DeleteRollupJobAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/action/DeleteRollupJobAction.java index 771f5e98d5c68..f77313b0f6bfd 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/action/DeleteRollupJobAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/action/DeleteRollupJobAction.java @@ -112,17 +112,5 @@ public Response() { public Response(boolean acknowledged) { super(acknowledged); } - - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/action/PutRollupJobAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/action/PutRollupJobAction.java index d4b7e33123054..c6e279f11db97 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/action/PutRollupJobAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/action/PutRollupJobAction.java @@ -150,17 +150,5 @@ public Response() { public Response(boolean acknowledged) { super(acknowledged); } - - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/watcher/transport/actions/service/WatcherServiceResponse.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/watcher/transport/actions/service/WatcherServiceResponse.java index 61ac4435f89e6..7df144e45d37e 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/watcher/transport/actions/service/WatcherServiceResponse.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/watcher/transport/actions/service/WatcherServiceResponse.java @@ -6,10 +6,6 @@ package org.elasticsearch.xpack.core.watcher.transport.actions.service; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; - -import java.io.IOException; public class WatcherServiceResponse extends AcknowledgedResponse { @@ -19,16 +15,4 @@ public WatcherServiceResponse() { public WatcherServiceResponse(boolean acknowledged) { super(acknowledged); } - - @Override - public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - readAcknowledged(in); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - super.writeTo(out); - writeAcknowledged(out); - } }