Skip to content

Commit bc57fe3

Browse files
nknizeashking94
nknize
authored and
ashking94
committed
[Remove] Deprecated serialization logic from pipeline aggs (opensearch-project#4847)
Removes the deprecated readTo/writeFrom serialization logic from piepline aggs that is no longer used as of Legacy version 7.8. Signed-off-by: Nicholas Walter Knize <[email protected]>
1 parent 3072fa2 commit bc57fe3

29 files changed

+22
-800
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
102102
- Always auto release the flood stage block ([#4703](https://github.com/opensearch-project/OpenSearch/pull/4703))
103103
- Remove LegacyESVersion.V_7_4_ and V_7_5_ Constants ([#4704](https://github.com/opensearch-project/OpenSearch/pull/4704))
104104
- Remove Legacy Version support from Snapshot/Restore Service ([#4728](https://github.com/opensearch-project/OpenSearch/pull/4728))
105+
- Remove deprecated serialization logic from pipeline aggs ([#4847](https://github.com/opensearch-project/OpenSearch/pull/4847))
105106

106107
### Fixed
107108
- `opensearch-service.bat start` and `opensearch-service.bat manager` failing to run ([#4289](https://github.com/opensearch-project/OpenSearch/pull/4289))

‎server/src/main/java/org/opensearch/plugins/SearchPlugin.java

+1-73
Original file line numberDiff line numberDiff line change
@@ -621,52 +621,6 @@ class PipelineAggregationSpec extends SearchExtensionSpec<
621621
PipelineAggregationBuilder,
622622
ContextParser<String, ? extends PipelineAggregationBuilder>> {
623623
private final Map<String, Writeable.Reader<? extends InternalAggregation>> resultReaders = new TreeMap<>();
624-
/**
625-
* Read the aggregator from a stream.
626-
* @deprecated Pipelines implemented after 7.8.0 do not need to be sent across the wire
627-
*/
628-
@Deprecated
629-
private final Writeable.Reader<? extends PipelineAggregator> aggregatorReader;
630-
631-
/**
632-
* Specification of a {@link PipelineAggregator}.
633-
*
634-
* @param name holds the names by which this aggregation might be parsed. The {@link ParseField#getPreferredName()} is special as it
635-
* is the name by under which the readers are registered. So it is the name that the {@link PipelineAggregationBuilder} and
636-
* {@link PipelineAggregator} should return from {@link NamedWriteable#getWriteableName()}. It is an error if
637-
* {@link ParseField#getPreferredName()} conflicts with another registered name, including names from other plugins.
638-
* @param builderReader the reader registered for this aggregation's builder. Typically, a reference to a constructor that takes a
639-
* {@link StreamInput}
640-
* @param parser reads the aggregation builder from XContent
641-
*/
642-
public PipelineAggregationSpec(
643-
ParseField name,
644-
Writeable.Reader<? extends PipelineAggregationBuilder> builderReader,
645-
ContextParser<String, ? extends PipelineAggregationBuilder> parser
646-
) {
647-
super(name, builderReader, parser);
648-
this.aggregatorReader = null;
649-
}
650-
651-
/**
652-
* Specification of a {@link PipelineAggregator}.
653-
*
654-
* @param name name by which this aggregation might be parsed or deserialized. Make sure it is the name that the
655-
* {@link PipelineAggregationBuilder} and {@link PipelineAggregator} should return from
656-
* {@link NamedWriteable#getWriteableName()}. It is an error if this name conflicts with another registered name, including
657-
* names from other plugins.
658-
* @param builderReader the reader registered for this aggregation's builder. Typically, a reference to a constructor that takes a
659-
* {@link StreamInput}
660-
* @param parser reads the aggregation builder from XContent
661-
*/
662-
public PipelineAggregationSpec(
663-
String name,
664-
Writeable.Reader<? extends PipelineAggregationBuilder> builderReader,
665-
ContextParser<String, ? extends PipelineAggregationBuilder> parser
666-
) {
667-
super(name, builderReader, parser);
668-
this.aggregatorReader = null;
669-
}
670624

671625
/**
672626
* Specification of a {@link PipelineAggregator}.
@@ -677,20 +631,14 @@ public PipelineAggregationSpec(
677631
* {@link ParseField#getPreferredName()} conflicts with another registered name, including names from other plugins.
678632
* @param builderReader the reader registered for this aggregation's builder. Typically, a reference to a constructor that takes a
679633
* {@link StreamInput}
680-
* @param aggregatorReader reads the {@link PipelineAggregator} from a stream
681634
* @param parser reads the aggregation builder from XContent
682-
* @deprecated Use {@link PipelineAggregationSpec#PipelineAggregationSpec(ParseField, Writeable.Reader, ContextParser)} for
683-
* pipelines implemented after 7.8.0
684635
*/
685-
@Deprecated
686636
public PipelineAggregationSpec(
687637
ParseField name,
688638
Writeable.Reader<? extends PipelineAggregationBuilder> builderReader,
689-
Writeable.Reader<? extends PipelineAggregator> aggregatorReader,
690639
ContextParser<String, ? extends PipelineAggregationBuilder> parser
691640
) {
692641
super(name, builderReader, parser);
693-
this.aggregatorReader = aggregatorReader;
694642
}
695643

696644
/**
@@ -702,20 +650,15 @@ public PipelineAggregationSpec(
702650
* names from other plugins.
703651
* @param builderReader the reader registered for this aggregation's builder. Typically, a reference to a constructor that takes a
704652
* {@link StreamInput}
705-
* @param aggregatorReader reads the {@link PipelineAggregator} from a stream
706653
* @param parser reads the aggregation builder from XContent
707-
* @deprecated Use {@link PipelineAggregationSpec#PipelineAggregationSpec(String, Writeable.Reader, ContextParser)} for pipelines
708-
* implemented after 7.8.0
654+
709655
*/
710-
@Deprecated
711656
public PipelineAggregationSpec(
712657
String name,
713658
Writeable.Reader<? extends PipelineAggregationBuilder> builderReader,
714-
Writeable.Reader<? extends PipelineAggregator> aggregatorReader,
715659
ContextParser<String, ? extends PipelineAggregationBuilder> parser
716660
) {
717661
super(name, builderReader, parser);
718-
this.aggregatorReader = aggregatorReader;
719662
}
720663

721664
/**
@@ -727,19 +670,16 @@ public PipelineAggregationSpec(
727670
* {@link ParseField#getPreferredName()} conflicts with another registered name, including names from other plugins.
728671
* @param builderReader the reader registered for this aggregation's builder. Typically, a reference to a constructor that takes a
729672
* {@link StreamInput}
730-
* @param aggregatorReader reads the {@link PipelineAggregator} from a stream
731673
* @param parser reads the aggregation builder from XContent
732674
* @deprecated prefer the ctor that takes a {@link ContextParser}
733675
*/
734676
@Deprecated
735677
public PipelineAggregationSpec(
736678
ParseField name,
737679
Writeable.Reader<? extends PipelineAggregationBuilder> builderReader,
738-
Writeable.Reader<? extends PipelineAggregator> aggregatorReader,
739680
PipelineAggregator.Parser parser
740681
) {
741682
super(name, builderReader, (p, n) -> parser.parse(n, p));
742-
this.aggregatorReader = aggregatorReader;
743683
}
744684

745685
/**
@@ -751,18 +691,15 @@ public PipelineAggregationSpec(
751691
* names from other plugins.
752692
* @param builderReader the reader registered for this aggregation's builder. Typically, a reference to a constructor that takes a
753693
* {@link StreamInput}
754-
* @param aggregatorReader reads the {@link PipelineAggregator} from a stream
755694
* @deprecated prefer the ctor that takes a {@link ContextParser}
756695
*/
757696
@Deprecated
758697
public PipelineAggregationSpec(
759698
String name,
760699
Writeable.Reader<? extends PipelineAggregationBuilder> builderReader,
761-
Writeable.Reader<? extends PipelineAggregator> aggregatorReader,
762700
PipelineAggregator.Parser parser
763701
) {
764702
super(name, builderReader, (p, n) -> parser.parse(n, p));
765-
this.aggregatorReader = aggregatorReader;
766703
}
767704

768705
/**
@@ -781,15 +718,6 @@ public PipelineAggregationSpec addResultReader(String writeableName, Writeable.R
781718
return this;
782719
}
783720

784-
/**
785-
* Read the aggregator from a stream.
786-
* @deprecated Pipelines implemented after 7.8.0 do not need to be sent across the wire
787-
*/
788-
@Deprecated
789-
public Writeable.Reader<? extends PipelineAggregator> getAggregatorReader() {
790-
return aggregatorReader;
791-
}
792-
793721
/**
794722
* Get the readers that must be registered for this aggregation's results.
795723
*/

‎server/src/main/java/org/opensearch/search/SearchModule.java

-36
Original file line numberDiff line numberDiff line change
@@ -211,21 +211,14 @@
211211
import org.opensearch.search.aggregations.metrics.ValueCountAggregationBuilder;
212212
import org.opensearch.search.aggregations.metrics.WeightedAvgAggregationBuilder;
213213
import org.opensearch.search.aggregations.pipeline.AvgBucketPipelineAggregationBuilder;
214-
import org.opensearch.search.aggregations.pipeline.AvgBucketPipelineAggregator;
215214
import org.opensearch.search.aggregations.pipeline.BucketScriptPipelineAggregationBuilder;
216-
import org.opensearch.search.aggregations.pipeline.BucketScriptPipelineAggregator;
217215
import org.opensearch.search.aggregations.pipeline.BucketSelectorPipelineAggregationBuilder;
218-
import org.opensearch.search.aggregations.pipeline.BucketSelectorPipelineAggregator;
219216
import org.opensearch.search.aggregations.pipeline.BucketSortPipelineAggregationBuilder;
220-
import org.opensearch.search.aggregations.pipeline.BucketSortPipelineAggregator;
221217
import org.opensearch.search.aggregations.pipeline.CumulativeSumPipelineAggregationBuilder;
222-
import org.opensearch.search.aggregations.pipeline.CumulativeSumPipelineAggregator;
223218
import org.opensearch.search.aggregations.pipeline.DerivativePipelineAggregationBuilder;
224-
import org.opensearch.search.aggregations.pipeline.DerivativePipelineAggregator;
225219
import org.opensearch.search.aggregations.pipeline.EwmaModel;
226220
import org.opensearch.search.aggregations.pipeline.ExtendedStatsBucketParser;
227221
import org.opensearch.search.aggregations.pipeline.ExtendedStatsBucketPipelineAggregationBuilder;
228-
import org.opensearch.search.aggregations.pipeline.ExtendedStatsBucketPipelineAggregator;
229222
import org.opensearch.search.aggregations.pipeline.HoltLinearModel;
230223
import org.opensearch.search.aggregations.pipeline.HoltWintersModel;
231224
import org.opensearch.search.aggregations.pipeline.InternalBucketMetricValue;
@@ -236,24 +229,15 @@
236229
import org.opensearch.search.aggregations.pipeline.InternalStatsBucket;
237230
import org.opensearch.search.aggregations.pipeline.LinearModel;
238231
import org.opensearch.search.aggregations.pipeline.MaxBucketPipelineAggregationBuilder;
239-
import org.opensearch.search.aggregations.pipeline.MaxBucketPipelineAggregator;
240232
import org.opensearch.search.aggregations.pipeline.MinBucketPipelineAggregationBuilder;
241-
import org.opensearch.search.aggregations.pipeline.MinBucketPipelineAggregator;
242233
import org.opensearch.search.aggregations.pipeline.MovAvgModel;
243234
import org.opensearch.search.aggregations.pipeline.MovAvgPipelineAggregationBuilder;
244-
import org.opensearch.search.aggregations.pipeline.MovAvgPipelineAggregator;
245235
import org.opensearch.search.aggregations.pipeline.MovFnPipelineAggregationBuilder;
246-
import org.opensearch.search.aggregations.pipeline.MovFnPipelineAggregator;
247236
import org.opensearch.search.aggregations.pipeline.PercentilesBucketPipelineAggregationBuilder;
248-
import org.opensearch.search.aggregations.pipeline.PercentilesBucketPipelineAggregator;
249-
import org.opensearch.search.aggregations.pipeline.PipelineAggregator;
250237
import org.opensearch.search.aggregations.pipeline.SerialDiffPipelineAggregationBuilder;
251-
import org.opensearch.search.aggregations.pipeline.SerialDiffPipelineAggregator;
252238
import org.opensearch.search.aggregations.pipeline.SimpleModel;
253239
import org.opensearch.search.aggregations.pipeline.StatsBucketPipelineAggregationBuilder;
254-
import org.opensearch.search.aggregations.pipeline.StatsBucketPipelineAggregator;
255240
import org.opensearch.search.aggregations.pipeline.SumBucketPipelineAggregationBuilder;
256-
import org.opensearch.search.aggregations.pipeline.SumBucketPipelineAggregator;
257241
import org.opensearch.search.aggregations.support.ValuesSourceRegistry;
258242
import org.opensearch.search.fetch.FetchPhase;
259243
import org.opensearch.search.fetch.FetchSubPhase;
@@ -710,15 +694,13 @@ private void registerPipelineAggregations(List<SearchPlugin> plugins) {
710694
new PipelineAggregationSpec(
711695
DerivativePipelineAggregationBuilder.NAME,
712696
DerivativePipelineAggregationBuilder::new,
713-
DerivativePipelineAggregator::new,
714697
DerivativePipelineAggregationBuilder::parse
715698
).addResultReader(InternalDerivative::new)
716699
);
717700
registerPipelineAggregation(
718701
new PipelineAggregationSpec(
719702
MaxBucketPipelineAggregationBuilder.NAME,
720703
MaxBucketPipelineAggregationBuilder::new,
721-
MaxBucketPipelineAggregator::new,
722704
MaxBucketPipelineAggregationBuilder.PARSER
723705
)
724706
// This bucket is used by many pipeline aggreations.
@@ -728,7 +710,6 @@ private void registerPipelineAggregations(List<SearchPlugin> plugins) {
728710
new PipelineAggregationSpec(
729711
MinBucketPipelineAggregationBuilder.NAME,
730712
MinBucketPipelineAggregationBuilder::new,
731-
MinBucketPipelineAggregator::new,
732713
MinBucketPipelineAggregationBuilder.PARSER
733714
)
734715
/* Uses InternalBucketMetricValue */
@@ -737,7 +718,6 @@ private void registerPipelineAggregations(List<SearchPlugin> plugins) {
737718
new PipelineAggregationSpec(
738719
AvgBucketPipelineAggregationBuilder.NAME,
739720
AvgBucketPipelineAggregationBuilder::new,
740-
AvgBucketPipelineAggregator::new,
741721
AvgBucketPipelineAggregationBuilder.PARSER
742722
)
743723
// This bucket is used by many pipeline aggreations.
@@ -747,7 +727,6 @@ private void registerPipelineAggregations(List<SearchPlugin> plugins) {
747727
new PipelineAggregationSpec(
748728
SumBucketPipelineAggregationBuilder.NAME,
749729
SumBucketPipelineAggregationBuilder::new,
750-
SumBucketPipelineAggregator::new,
751730
SumBucketPipelineAggregationBuilder.PARSER
752731
)
753732
/* Uses InternalSimpleValue */
@@ -756,31 +735,27 @@ private void registerPipelineAggregations(List<SearchPlugin> plugins) {
756735
new PipelineAggregationSpec(
757736
StatsBucketPipelineAggregationBuilder.NAME,
758737
StatsBucketPipelineAggregationBuilder::new,
759-
StatsBucketPipelineAggregator::new,
760738
StatsBucketPipelineAggregationBuilder.PARSER
761739
).addResultReader(InternalStatsBucket::new)
762740
);
763741
registerPipelineAggregation(
764742
new PipelineAggregationSpec(
765743
ExtendedStatsBucketPipelineAggregationBuilder.NAME,
766744
ExtendedStatsBucketPipelineAggregationBuilder::new,
767-
ExtendedStatsBucketPipelineAggregator::new,
768745
new ExtendedStatsBucketParser()
769746
).addResultReader(InternalExtendedStatsBucket::new)
770747
);
771748
registerPipelineAggregation(
772749
new PipelineAggregationSpec(
773750
PercentilesBucketPipelineAggregationBuilder.NAME,
774751
PercentilesBucketPipelineAggregationBuilder::new,
775-
PercentilesBucketPipelineAggregator::new,
776752
PercentilesBucketPipelineAggregationBuilder.PARSER
777753
).addResultReader(InternalPercentilesBucket::new)
778754
);
779755
registerPipelineAggregation(
780756
new PipelineAggregationSpec(
781757
MovAvgPipelineAggregationBuilder.NAME,
782758
MovAvgPipelineAggregationBuilder::new,
783-
MovAvgPipelineAggregator::new,
784759
(XContentParser parser, String name) -> MovAvgPipelineAggregationBuilder.parse(
785760
movingAverageModelParserRegistry,
786761
name,
@@ -792,47 +767,41 @@ private void registerPipelineAggregations(List<SearchPlugin> plugins) {
792767
new PipelineAggregationSpec(
793768
CumulativeSumPipelineAggregationBuilder.NAME,
794769
CumulativeSumPipelineAggregationBuilder::new,
795-
CumulativeSumPipelineAggregator::new,
796770
CumulativeSumPipelineAggregationBuilder.PARSER
797771
)
798772
);
799773
registerPipelineAggregation(
800774
new PipelineAggregationSpec(
801775
BucketScriptPipelineAggregationBuilder.NAME,
802776
BucketScriptPipelineAggregationBuilder::new,
803-
BucketScriptPipelineAggregator::new,
804777
BucketScriptPipelineAggregationBuilder.PARSER
805778
)
806779
);
807780
registerPipelineAggregation(
808781
new PipelineAggregationSpec(
809782
BucketSelectorPipelineAggregationBuilder.NAME,
810783
BucketSelectorPipelineAggregationBuilder::new,
811-
BucketSelectorPipelineAggregator::new,
812784
BucketSelectorPipelineAggregationBuilder::parse
813785
)
814786
);
815787
registerPipelineAggregation(
816788
new PipelineAggregationSpec(
817789
BucketSortPipelineAggregationBuilder.NAME,
818790
BucketSortPipelineAggregationBuilder::new,
819-
BucketSortPipelineAggregator::new,
820791
BucketSortPipelineAggregationBuilder::parse
821792
)
822793
);
823794
registerPipelineAggregation(
824795
new PipelineAggregationSpec(
825796
SerialDiffPipelineAggregationBuilder.NAME,
826797
SerialDiffPipelineAggregationBuilder::new,
827-
SerialDiffPipelineAggregator::new,
828798
SerialDiffPipelineAggregationBuilder::parse
829799
)
830800
);
831801
registerPipelineAggregation(
832802
new PipelineAggregationSpec(
833803
MovFnPipelineAggregationBuilder.NAME,
834804
MovFnPipelineAggregationBuilder::new,
835-
MovFnPipelineAggregator::new,
836805
MovFnPipelineAggregationBuilder.PARSER
837806
)
838807
);
@@ -847,11 +816,6 @@ private void registerPipelineAggregation(PipelineAggregationSpec spec) {
847816
namedWriteables.add(
848817
new NamedWriteableRegistry.Entry(PipelineAggregationBuilder.class, spec.getName().getPreferredName(), spec.getReader())
849818
);
850-
if (spec.getAggregatorReader() != null) {
851-
namedWriteables.add(
852-
new NamedWriteableRegistry.Entry(PipelineAggregator.class, spec.getName().getPreferredName(), spec.getAggregatorReader())
853-
);
854-
}
855819
for (Map.Entry<String, Writeable.Reader<? extends InternalAggregation>> resultReader : spec.getResultReaders().entrySet()) {
856820
namedWriteables.add(
857821
new NamedWriteableRegistry.Entry(InternalAggregation.class, resultReader.getKey(), resultReader.getValue())

‎server/src/main/java/org/opensearch/search/aggregations/AggregationPhase.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,7 @@ public void execute(SearchContext context) {
148148
throw new AggregationExecutionException("Failed to build aggregation [" + aggregator.name() + "]", e);
149149
}
150150
}
151-
context.queryResult()
152-
.aggregations(new InternalAggregations(aggregations, context.request().source().aggregations()::buildPipelineTree));
151+
context.queryResult().aggregations(new InternalAggregations(aggregations));
153152

154153
// disable aggregations so that they don't run on next pages in case of scrolling
155154
context.aggregations(null);

0 commit comments

Comments
 (0)