From 1f3abe527b65e6d0045dde661e952627f14ddf3c Mon Sep 17 00:00:00 2001 From: Finn Carroll Date: Thu, 22 Aug 2024 13:54:03 -0700 Subject: [PATCH] Move FetchSearchResults serialization to FetchSearchResultsSerDe Signed-off-by: Finn Carroll --- .../org/opensearch/search/SearchHits.java | 2 +- .../search/fetch/FetchSearchResult.java | 28 +++++++++++++--- .../fetch/serde/FetchSearchResultsSerDe.java | 32 +++++++++++++++++-- 3 files changed, 54 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/opensearch/search/SearchHits.java b/server/src/main/java/org/opensearch/search/SearchHits.java index b5dd3709cfbcc..8839c31ca21b8 100644 --- a/server/src/main/java/org/opensearch/search/SearchHits.java +++ b/server/src/main/java/org/opensearch/search/SearchHits.java @@ -171,7 +171,7 @@ public SearchHits(StreamInput in) throws IOException { /** * Preserving for compatibility. - * Going forward deserialize with dedicated SearchHitSerDe. + * Going forward deserialize with dedicated SearchHitsSerDe. */ @Override public void writeTo(StreamOutput out) throws IOException { diff --git a/server/src/main/java/org/opensearch/search/fetch/FetchSearchResult.java b/server/src/main/java/org/opensearch/search/fetch/FetchSearchResult.java index 817f57be21306..25a72cbc8f103 100644 --- a/server/src/main/java/org/opensearch/search/fetch/FetchSearchResult.java +++ b/server/src/main/java/org/opensearch/search/fetch/FetchSearchResult.java @@ -40,6 +40,8 @@ import org.opensearch.search.SearchHits; import org.opensearch.search.SearchPhaseResult; import org.opensearch.search.SearchShardTarget; +import org.opensearch.search.fetch.serde.FetchSearchResultsSerDe; +import org.opensearch.search.fetch.serde.SearchHitsSerDe; import org.opensearch.search.internal.ShardSearchContextId; import org.opensearch.search.query.QuerySearchResult; @@ -59,10 +61,22 @@ public class FetchSearchResult extends SearchPhaseResult { public FetchSearchResult() {} + /** + * Preserving for compatibility. + * Going forward deserialize with dedicated FetchSearchResultsSerDe. + */ public FetchSearchResult(StreamInput in) throws IOException { - super(in); - contextId = new ShardSearchContextId(in); - hits = new SearchHits(in); + this(new FetchSearchResultsSerDe().deserialize(in)); + } + + public FetchSearchResult(FetchSearchResult result) { + this.contextId = result.contextId; + this.hits = result.hits; + } + + public FetchSearchResult(ShardSearchContextId id, SearchHits hits) throws IOException { + this.contextId = id; + this.hits = hits; } public FetchSearchResult(ShardSearchContextId id, SearchShardTarget shardTarget) { @@ -128,9 +142,13 @@ public int counterGetAndIncrement() { return counter++; } + /** + * Preserving for compatibility. + * Going forward serialize with dedicated FetchSearchResultsSerDe. + */ @Override public void writeTo(StreamOutput out) throws IOException { - contextId.writeTo(out); - hits.writeTo(out); + FetchSearchResultsSerDe serDe = new FetchSearchResultsSerDe(); + serDe.serialize(this, out); } } diff --git a/server/src/main/java/org/opensearch/search/fetch/serde/FetchSearchResultsSerDe.java b/server/src/main/java/org/opensearch/search/fetch/serde/FetchSearchResultsSerDe.java index b1f9fcbe18003..5433645658b10 100644 --- a/server/src/main/java/org/opensearch/search/fetch/serde/FetchSearchResultsSerDe.java +++ b/server/src/main/java/org/opensearch/search/fetch/serde/FetchSearchResultsSerDe.java @@ -10,17 +10,30 @@ import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.search.SearchHits; import org.opensearch.search.fetch.FetchSearchResult; +import org.opensearch.search.internal.ShardSearchContextId; import java.io.IOException; public class FetchSearchResultsSerDe implements SerDe.StreamSerializer, SerDe.StreamDeserializer { + /** + * TODO NOTE: FetchSearchResult inheritance structure is as follows. + * TransportMessage -> TransportResponse -> SearchPhaseResult -> FetchSearchResult. + * Serialization of parent classes is currently a no-op. + * For completeness these parent classes should be mirrored here respectively with: + * TransportMessageSerDe, TransportResponseSerDe, SearchPhaseResultSerDe. + * However, currently only SearchHitsSerDe is needed for serialization. + * + * This is implicitely enforced by FetchSearchResult as well on the serialization side. + * writeTo doesn't call a parent implementation... + */ SearchHitsSerDe searchHitsSerDe; @Override public FetchSearchResult deserialize(StreamInput in) { try { - return new FetchSearchResult(in); + return fromStream(in); } catch (IOException e) { throw new SerDe.SerializationException("Failed to deserialize FetchSearchResult", e); } @@ -29,9 +42,24 @@ public FetchSearchResult deserialize(StreamInput in) { @Override public void serialize(FetchSearchResult object, StreamOutput out) throws SerDe.SerializationException { try { - object.writeTo(out); + toStream(object, out); } catch (IOException e) { throw new SerDe.SerializationException("Failed to serialize FetchSearchResult", e); } } + + private FetchSearchResult fromStream(StreamInput in) throws IOException { + ShardSearchContextId contextId = new ShardSearchContextId(in); + SearchHits hits = new SearchHits(in); + return new FetchSearchResult(contextId, hits); + } + + private void toStream(FetchSearchResult object, StreamOutput out) throws IOException { + FetchSearchResult.SerializationAccess serI = object.getSerAccess(); + ShardSearchContextId contextId = serI.getShardSearchContextId(); + SearchHits hits = serI.getHits(); + + contextId.writeTo(out); + hits.writeTo(out); + } }