From b7c7f64c156311ea1e99baaf3011de6c335679fb Mon Sep 17 00:00:00 2001 From: SwethaGuptha <156877431+SwethaGuptha@users.noreply.github.com> Date: Wed, 4 Sep 2024 14:04:13 +0530 Subject: [PATCH 1/8] Reset discovery nodes in most transport node actions request. (#15131) Signed-off-by: Swetha Guptha Co-authored-by: Swetha Guptha --- CHANGELOG.md | 1 + .../hotthreads/NodesHotThreadsRequest.java | 2 +- .../cluster/node/info/NodesInfoRequest.java | 2 +- .../cluster/node/stats/NodesStatsRequest.java | 4 +- .../cluster/node/usage/NodesUsageRequest.java | 2 +- .../status/TransportNodesSnapshotsStatus.java | 2 +- .../cluster/stats/ClusterStatsRequest.java | 2 +- .../find/FindDanglingIndexRequest.java | 2 +- .../list/ListDanglingIndicesRequest.java | 4 +- .../action/search/GetAllPitNodesRequest.java | 2 +- .../support/nodes/BaseNodesRequest.java | 16 ++++-- .../support/nodes/TransportNodesAction.java | 12 ++--- .../TransportNodesListGatewayMetaState.java | 2 +- ...ransportNodesListGatewayStartedShards.java | 2 +- ...ortNodesListGatewayStartedShardsBatch.java | 2 +- .../TransportNodesListShardStoreMetadata.java | 2 +- ...sportNodesListShardStoreMetadataBatch.java | 2 +- .../admin/cluster/RestClusterStatsAction.java | 1 - .../admin/cluster/RestNodesInfoAction.java | 1 - .../admin/cluster/RestNodesStatsAction.java | 1 - .../rest/action/cat/RestNodesAction.java | 2 - .../TransportClusterStatsActionTests.java | 37 ------------- .../nodes/TransportNodesActionTests.java | 52 ++++++++++++++++++- .../nodes/TransportNodesInfoActionTests.java | 20 ------- .../nodes/TransportNodesStatsActionTests.java | 24 +-------- 25 files changed, 87 insertions(+), 112 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa25dc88a64d7..0e255ad694206 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Remote Publication] Added checksum validation for cluster state behind a cluster setting ([#15218](https://github.com/opensearch-project/OpenSearch/pull/15218)) - Add canRemain method to TargetPoolAllocationDecider to move shards from local to remote pool for hot to warm tiering ([#15010](https://github.com/opensearch-project/OpenSearch/pull/15010)) - ClusterManagerTaskThrottler Improvements ([#15508](https://github.com/opensearch-project/OpenSearch/pull/15508)) +- Reset DiscoveryNodes in all transport node actions request ([#15131](https://github.com/opensearch-project/OpenSearch/pull/15131)) ### Dependencies - Bump `netty` from 4.1.111.Final to 4.1.112.Final ([#15081](https://github.com/opensearch-project/OpenSearch/pull/15081)) diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/node/hotthreads/NodesHotThreadsRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/node/hotthreads/NodesHotThreadsRequest.java index 9e52b90f7bd38..b7b60732fcbfc 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/node/hotthreads/NodesHotThreadsRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/node/hotthreads/NodesHotThreadsRequest.java @@ -70,7 +70,7 @@ public NodesHotThreadsRequest(StreamInput in) throws IOException { * threads for all nodes is used. */ public NodesHotThreadsRequest(String... nodesIds) { - super(nodesIds); + super(false, nodesIds); } public int threads() { diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/node/info/NodesInfoRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/node/info/NodesInfoRequest.java index 17b633c533218..26b53e8db642f 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/node/info/NodesInfoRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/node/info/NodesInfoRequest.java @@ -72,7 +72,7 @@ public NodesInfoRequest(StreamInput in) throws IOException { * for all nodes will be returned. */ public NodesInfoRequest(String... nodesIds) { - super(nodesIds); + super(false, nodesIds); defaultMetrics(); } diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodesStatsRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodesStatsRequest.java index 379836cf442e3..f1f9f93afdad2 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodesStatsRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/node/stats/NodesStatsRequest.java @@ -58,7 +58,7 @@ public class NodesStatsRequest extends BaseNodesRequest { private final Set requestedMetrics = new HashSet<>(); public NodesStatsRequest() { - super((String[]) null); + super(false, (String[]) null); } public NodesStatsRequest(StreamInput in) throws IOException { @@ -74,7 +74,7 @@ public NodesStatsRequest(StreamInput in) throws IOException { * for all nodes will be returned. */ public NodesStatsRequest(String... nodesIds) { - super(nodesIds); + super(false, nodesIds); } /** diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/node/usage/NodesUsageRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/node/usage/NodesUsageRequest.java index 385f48d5690c1..955f77921009f 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/node/usage/NodesUsageRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/node/usage/NodesUsageRequest.java @@ -61,7 +61,7 @@ public NodesUsageRequest(StreamInput in) throws IOException { * passed, usage for all nodes will be returned. */ public NodesUsageRequest(String... nodesIds) { - super(nodesIds); + super(false, nodesIds); } /** diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/status/TransportNodesSnapshotsStatus.java b/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/status/TransportNodesSnapshotsStatus.java index 8f76dbce048cd..3654cab3c5a47 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/status/TransportNodesSnapshotsStatus.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/status/TransportNodesSnapshotsStatus.java @@ -161,7 +161,7 @@ public Request(StreamInput in) throws IOException { } public Request(String[] nodesIds) { - super(nodesIds); + super(false, nodesIds); } public Request snapshots(Snapshot[] snapshots) { diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsRequest.java index bd75b2210e474..b82a9d256a134 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/stats/ClusterStatsRequest.java @@ -62,7 +62,7 @@ public ClusterStatsRequest(StreamInput in) throws IOException { * based on all nodes will be returned. */ public ClusterStatsRequest(String... nodesIds) { - super(nodesIds); + super(false, nodesIds); } public boolean useAggregatedNodeLevelResponses() { diff --git a/server/src/main/java/org/opensearch/action/admin/indices/dangling/find/FindDanglingIndexRequest.java b/server/src/main/java/org/opensearch/action/admin/indices/dangling/find/FindDanglingIndexRequest.java index f853a47b3c2bf..b38cb46b23ace 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/dangling/find/FindDanglingIndexRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/dangling/find/FindDanglingIndexRequest.java @@ -53,7 +53,7 @@ public FindDanglingIndexRequest(StreamInput in) throws IOException { } public FindDanglingIndexRequest(String indexUUID) { - super(Strings.EMPTY_ARRAY); + super(false, Strings.EMPTY_ARRAY); this.indexUUID = indexUUID; } diff --git a/server/src/main/java/org/opensearch/action/admin/indices/dangling/list/ListDanglingIndicesRequest.java b/server/src/main/java/org/opensearch/action/admin/indices/dangling/list/ListDanglingIndicesRequest.java index 119c4acbf4160..0076c9e294b69 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/dangling/list/ListDanglingIndicesRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/dangling/list/ListDanglingIndicesRequest.java @@ -58,12 +58,12 @@ public ListDanglingIndicesRequest(StreamInput in) throws IOException { } public ListDanglingIndicesRequest() { - super(Strings.EMPTY_ARRAY); + super(false, Strings.EMPTY_ARRAY); this.indexUUID = null; } public ListDanglingIndicesRequest(String indexUUID) { - super(Strings.EMPTY_ARRAY); + super(false, Strings.EMPTY_ARRAY); this.indexUUID = indexUUID; } diff --git a/server/src/main/java/org/opensearch/action/search/GetAllPitNodesRequest.java b/server/src/main/java/org/opensearch/action/search/GetAllPitNodesRequest.java index 336c8139561e9..97c0d66f4b8e1 100644 --- a/server/src/main/java/org/opensearch/action/search/GetAllPitNodesRequest.java +++ b/server/src/main/java/org/opensearch/action/search/GetAllPitNodesRequest.java @@ -27,7 +27,7 @@ public class GetAllPitNodesRequest extends BaseNodesRequest * Setting default behavior as `true` but can be explicitly changed in requests that do not require. */ private boolean includeDiscoveryNodes = true; + private final TimeValue DEFAULT_TIMEOUT_SECS = TimeValue.timeValueSeconds(30); private TimeValue timeout; @@ -88,11 +89,22 @@ protected BaseNodesRequest(String... nodesIds) { this.nodesIds = nodesIds; } + protected BaseNodesRequest(boolean includeDiscoveryNodes, String... nodesIds) { + this.nodesIds = nodesIds; + this.includeDiscoveryNodes = includeDiscoveryNodes; + } + protected BaseNodesRequest(DiscoveryNode... concreteNodes) { this.nodesIds = null; this.concreteNodes = concreteNodes; } + protected BaseNodesRequest(boolean includeDiscoveryNodes, DiscoveryNode... concreteNodes) { + this.nodesIds = null; + this.concreteNodes = concreteNodes; + this.includeDiscoveryNodes = includeDiscoveryNodes; + } + public final String[] nodesIds() { return nodesIds; } @@ -127,10 +139,6 @@ public void setConcreteNodes(DiscoveryNode[] concreteNodes) { this.concreteNodes = concreteNodes; } - public void setIncludeDiscoveryNodes(boolean value) { - includeDiscoveryNodes = value; - } - public boolean getIncludeDiscoveryNodes() { return includeDiscoveryNodes; } diff --git a/server/src/main/java/org/opensearch/action/support/nodes/TransportNodesAction.java b/server/src/main/java/org/opensearch/action/support/nodes/TransportNodesAction.java index 3acd12f632e0f..dccd5059dd52d 100644 --- a/server/src/main/java/org/opensearch/action/support/nodes/TransportNodesAction.java +++ b/server/src/main/java/org/opensearch/action/support/nodes/TransportNodesAction.java @@ -240,18 +240,16 @@ class AsyncAction { } this.responses = new AtomicReferenceArray<>(request.concreteNodes().length); this.concreteNodes = request.concreteNodes(); - if (request.getIncludeDiscoveryNodes() == false) { - // As we transfer the ownership of discovery nodes to route the request to into the AsyncAction class, we - // remove the list of DiscoveryNodes from the request. This reduces the payload of the request and improves + // As we transfer the ownership of discovery nodes to route the request to into the AsyncAction class, + // we remove the list of DiscoveryNodes from the request. This reduces the payload of the request and improves // the number of concrete nodes in the memory. request.setConcreteNodes(null); } } void start() { - final DiscoveryNode[] nodes = this.concreteNodes; - if (nodes.length == 0) { + if (this.concreteNodes.length == 0) { // nothing to notify threadPool.generic().execute(() -> listener.onResponse(newResponse(request, responses))); return; @@ -260,9 +258,9 @@ void start() { if (request.timeout() != null) { builder.withTimeout(request.timeout()); } - for (int i = 0; i < nodes.length; i++) { + for (int i = 0; i < this.concreteNodes.length; i++) { final int idx = i; - final DiscoveryNode node = nodes[i]; + final DiscoveryNode node = this.concreteNodes[i]; final String nodeId = node.getId(); try { TransportRequest nodeRequest = newNodeRequest(request); diff --git a/server/src/main/java/org/opensearch/gateway/TransportNodesListGatewayMetaState.java b/server/src/main/java/org/opensearch/gateway/TransportNodesListGatewayMetaState.java index 647e3632ea0ca..7f634534d1ed6 100644 --- a/server/src/main/java/org/opensearch/gateway/TransportNodesListGatewayMetaState.java +++ b/server/src/main/java/org/opensearch/gateway/TransportNodesListGatewayMetaState.java @@ -133,7 +133,7 @@ public Request(StreamInput in) throws IOException { } public Request(String... nodesIds) { - super(nodesIds); + super(false, nodesIds); } } diff --git a/server/src/main/java/org/opensearch/gateway/TransportNodesListGatewayStartedShards.java b/server/src/main/java/org/opensearch/gateway/TransportNodesListGatewayStartedShards.java index 4b1f611bb88ab..b9cfada1910ce 100644 --- a/server/src/main/java/org/opensearch/gateway/TransportNodesListGatewayStartedShards.java +++ b/server/src/main/java/org/opensearch/gateway/TransportNodesListGatewayStartedShards.java @@ -197,7 +197,7 @@ public Request(StreamInput in) throws IOException { } public Request(ShardId shardId, String customDataPath, DiscoveryNode[] nodes) { - super(nodes); + super(false, nodes); this.shardId = Objects.requireNonNull(shardId); this.customDataPath = Objects.requireNonNull(customDataPath); } diff --git a/server/src/main/java/org/opensearch/gateway/TransportNodesListGatewayStartedShardsBatch.java b/server/src/main/java/org/opensearch/gateway/TransportNodesListGatewayStartedShardsBatch.java index 89362988b4d85..45ead58f9f195 100644 --- a/server/src/main/java/org/opensearch/gateway/TransportNodesListGatewayStartedShardsBatch.java +++ b/server/src/main/java/org/opensearch/gateway/TransportNodesListGatewayStartedShardsBatch.java @@ -182,7 +182,7 @@ public Request(StreamInput in) throws IOException { } public Request(DiscoveryNode[] nodes, Map shardAttributes) { - super(nodes); + super(false, nodes); this.shardAttributes = Objects.requireNonNull(shardAttributes); } diff --git a/server/src/main/java/org/opensearch/indices/store/TransportNodesListShardStoreMetadata.java b/server/src/main/java/org/opensearch/indices/store/TransportNodesListShardStoreMetadata.java index eeee5d8a409aa..2f666f67280bd 100644 --- a/server/src/main/java/org/opensearch/indices/store/TransportNodesListShardStoreMetadata.java +++ b/server/src/main/java/org/opensearch/indices/store/TransportNodesListShardStoreMetadata.java @@ -176,7 +176,7 @@ public Request(StreamInput in) throws IOException { } public Request(ShardId shardId, String customDataPath, DiscoveryNode[] nodes) { - super(nodes); + super(false, nodes); this.shardId = Objects.requireNonNull(shardId); this.customDataPath = Objects.requireNonNull(customDataPath); } diff --git a/server/src/main/java/org/opensearch/indices/store/TransportNodesListShardStoreMetadataBatch.java b/server/src/main/java/org/opensearch/indices/store/TransportNodesListShardStoreMetadataBatch.java index 22b03539cca74..2c84ec31b987d 100644 --- a/server/src/main/java/org/opensearch/indices/store/TransportNodesListShardStoreMetadataBatch.java +++ b/server/src/main/java/org/opensearch/indices/store/TransportNodesListShardStoreMetadataBatch.java @@ -188,7 +188,7 @@ public Request(StreamInput in) throws IOException { } public Request(Map shardAttributes, DiscoveryNode[] nodes) { - super(nodes); + super(false, nodes); this.shardAttributes = Objects.requireNonNull(shardAttributes); } diff --git a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestClusterStatsAction.java b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestClusterStatsAction.java index d4426a004af8e..ee33bd18db05d 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestClusterStatsAction.java +++ b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestClusterStatsAction.java @@ -66,7 +66,6 @@ public String getName() { public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { ClusterStatsRequest clusterStatsRequest = new ClusterStatsRequest().nodesIds(request.paramAsStringArray("nodeId", null)); clusterStatsRequest.timeout(request.param("timeout")); - clusterStatsRequest.setIncludeDiscoveryNodes(false); clusterStatsRequest.useAggregatedNodeLevelResponses(true); return channel -> client.admin().cluster().clusterStats(clusterStatsRequest, new NodesResponseRestListener<>(channel)); } diff --git a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestNodesInfoAction.java b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestNodesInfoAction.java index 4ac51933ea382..37e4c8783d0df 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestNodesInfoAction.java +++ b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestNodesInfoAction.java @@ -88,7 +88,6 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC final NodesInfoRequest nodesInfoRequest = prepareRequest(request); nodesInfoRequest.timeout(request.param("timeout")); settingsFilter.addFilterSettingParams(request); - nodesInfoRequest.setIncludeDiscoveryNodes(false); return channel -> client.admin().cluster().nodesInfo(nodesInfoRequest, new NodesResponseRestListener<>(channel)); } diff --git a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestNodesStatsAction.java b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestNodesStatsAction.java index 0119731e4a0d7..0a0ee84b1999f 100644 --- a/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestNodesStatsAction.java +++ b/server/src/main/java/org/opensearch/rest/action/admin/cluster/RestNodesStatsAction.java @@ -232,7 +232,6 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC // If no levels are passed in this results in an empty array. String[] levels = Strings.splitStringByCommaToArray(request.param("level")); nodesStatsRequest.indices().setLevels(levels); - nodesStatsRequest.setIncludeDiscoveryNodes(false); nodesStatsRequest.indices().setIncludeIndicesStatsByLevel(true); return channel -> client.admin().cluster().nodesStats(nodesStatsRequest, new NodesResponseRestListener<>(channel)); diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java index 1aa40b50290cd..26d6b91882cbd 100644 --- a/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java +++ b/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java @@ -125,7 +125,6 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli public void processResponse(final ClusterStateResponse clusterStateResponse) { NodesInfoRequest nodesInfoRequest = new NodesInfoRequest(); nodesInfoRequest.timeout(request.param("timeout")); - nodesInfoRequest.setIncludeDiscoveryNodes(false); nodesInfoRequest.clear() .addMetrics( NodesInfoRequest.Metric.JVM.metricName(), @@ -138,7 +137,6 @@ public void processResponse(final ClusterStateResponse clusterStateResponse) { public void processResponse(final NodesInfoResponse nodesInfoResponse) { NodesStatsRequest nodesStatsRequest = new NodesStatsRequest(); nodesStatsRequest.timeout(request.param("timeout")); - nodesStatsRequest.setIncludeDiscoveryNodes(false); nodesStatsRequest.clear() .indices(true) .addMetrics( diff --git a/server/src/test/java/org/opensearch/action/support/nodes/TransportClusterStatsActionTests.java b/server/src/test/java/org/opensearch/action/support/nodes/TransportClusterStatsActionTests.java index f8e14b477b8ef..fc920b847cef9 100644 --- a/server/src/test/java/org/opensearch/action/support/nodes/TransportClusterStatsActionTests.java +++ b/server/src/test/java/org/opensearch/action/support/nodes/TransportClusterStatsActionTests.java @@ -33,48 +33,12 @@ public class TransportClusterStatsActionTests extends TransportNodesActionTests { - /** - * By default, we send discovery nodes list to each request that is sent across from the coordinator node. This - * behavior is asserted in this test. - */ - public void testClusterStatsActionWithRetentionOfDiscoveryNodesList() { - ClusterStatsRequest request = new ClusterStatsRequest(); - request.setIncludeDiscoveryNodes(true); - Map> combinedSentRequest = performNodesInfoAction(request); - - assertNotNull(combinedSentRequest); - combinedSentRequest.forEach((node, capturedRequestList) -> { - assertNotNull(capturedRequestList); - capturedRequestList.forEach(sentRequest -> { - assertNotNull(sentRequest.getDiscoveryNodes()); - assertEquals(sentRequest.getDiscoveryNodes().length, clusterService.state().nodes().getSize()); - }); - }); - } - - public void testClusterStatsActionWithPreFilledConcreteNodesAndWithRetentionOfDiscoveryNodesList() { - ClusterStatsRequest request = new ClusterStatsRequest(); - Collection discoveryNodes = clusterService.state().getNodes().getNodes().values(); - request.setConcreteNodes(discoveryNodes.toArray(DiscoveryNode[]::new)); - Map> combinedSentRequest = performNodesInfoAction(request); - - assertNotNull(combinedSentRequest); - combinedSentRequest.forEach((node, capturedRequestList) -> { - assertNotNull(capturedRequestList); - capturedRequestList.forEach(sentRequest -> { - assertNotNull(sentRequest.getDiscoveryNodes()); - assertEquals(sentRequest.getDiscoveryNodes().length, clusterService.state().nodes().getSize()); - }); - }); - } - /** * In the optimized ClusterStats Request, we do not send the DiscoveryNodes List to each node. This behavior is * asserted in this test. */ public void testClusterStatsActionWithoutRetentionOfDiscoveryNodesList() { ClusterStatsRequest request = new ClusterStatsRequest(); - request.setIncludeDiscoveryNodes(false); Map> combinedSentRequest = performNodesInfoAction(request); assertNotNull(combinedSentRequest); @@ -88,7 +52,6 @@ public void testClusterStatsActionWithPreFilledConcreteNodesAndWithoutRetentionO ClusterStatsRequest request = new ClusterStatsRequest(); Collection discoveryNodes = clusterService.state().getNodes().getNodes().values(); request.setConcreteNodes(discoveryNodes.toArray(DiscoveryNode[]::new)); - request.setIncludeDiscoveryNodes(false); Map> combinedSentRequest = performNodesInfoAction(request); assertNotNull(combinedSentRequest); diff --git a/server/src/test/java/org/opensearch/action/support/nodes/TransportNodesActionTests.java b/server/src/test/java/org/opensearch/action/support/nodes/TransportNodesActionTests.java index 7e968aa8fb199..a338e68276bbc 100644 --- a/server/src/test/java/org/opensearch/action/support/nodes/TransportNodesActionTests.java +++ b/server/src/test/java/org/opensearch/action/support/nodes/TransportNodesActionTests.java @@ -62,6 +62,7 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -71,6 +72,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReferenceArray; import java.util.function.Supplier; +import java.util.stream.Collectors; import static org.opensearch.test.ClusterServiceUtils.createClusterService; import static org.opensearch.test.ClusterServiceUtils.setState; @@ -166,6 +168,36 @@ public void testCustomResolving() throws Exception { assertEquals(clusterService.state().nodes().getDataNodes().size(), capturedRequests.size()); } + public void testTransportNodesActionWithDiscoveryNodesIncluded() { + String[] nodeIds = clusterService.state().nodes().getNodes().keySet().toArray(new String[0]); + TestNodesRequest request = new TestNodesRequest(true, nodeIds); + getTestTransportNodesAction().new AsyncAction(null, request, new PlainActionFuture<>()).start(); + Map> capturedRequests = transport.getCapturedRequestsByTargetNodeAndClear(); + List capturedTransportNodeRequestList = capturedRequests.values() + .stream() + .flatMap(Collection::stream) + .map(capturedRequest -> (TestNodeRequest) capturedRequest.request) + .collect(Collectors.toList()); + assertEquals(nodeIds.length, capturedTransportNodeRequestList.size()); + capturedTransportNodeRequestList.forEach( + capturedRequest -> assertEquals(nodeIds.length, capturedRequest.testNodesRequest.concreteNodes().length) + ); + } + + public void testTransportNodesActionWithDiscoveryNodesReset() { + String[] nodeIds = clusterService.state().nodes().getNodes().keySet().toArray(new String[0]); + TestNodesRequest request = new TestNodesRequest(false, nodeIds); + getTestTransportNodesAction().new AsyncAction(null, request, new PlainActionFuture<>()).start(); + Map> capturedRequests = transport.getCapturedRequestsByTargetNodeAndClear(); + List capturedTransportNodeRequestList = capturedRequests.values() + .stream() + .flatMap(Collection::stream) + .map(capturedRequest -> (TestNodeRequest) capturedRequest.request) + .collect(Collectors.toList()); + assertEquals(nodeIds.length, capturedTransportNodeRequestList.size()); + capturedTransportNodeRequestList.forEach(capturedRequest -> assertNull(capturedRequest.testNodesRequest.concreteNodes())); + } + private List mockList(Supplier supplier, int size) { List failures = new ArrayList<>(size); for (int i = 0; i < size; ++i) { @@ -314,7 +346,7 @@ protected TestNodesResponse newResponse( @Override protected TestNodeRequest newNodeRequest(TestNodesRequest request) { - return new TestNodeRequest(); + return new TestNodeRequest(request); } @Override @@ -357,6 +389,10 @@ private static class TestNodesRequest extends BaseNodesRequest TestNodesRequest(String... nodesIds) { super(nodesIds); } + + TestNodesRequest(boolean includeDiscoveryNodes, String... nodesIds) { + super(includeDiscoveryNodes, nodesIds); + } } private static class TestNodesResponse extends BaseNodesResponse { @@ -385,10 +421,24 @@ protected void writeNodesTo(StreamOutput out, List nodes) thro } private static class TestNodeRequest extends TransportRequest { + + protected TestNodesRequest testNodesRequest; + TestNodeRequest() {} + TestNodeRequest(TestNodesRequest testNodesRequest) { + this.testNodesRequest = testNodesRequest; + } + TestNodeRequest(StreamInput in) throws IOException { super(in); + testNodesRequest = new TestNodesRequest(in); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + testNodesRequest.writeTo(out); } } diff --git a/server/src/test/java/org/opensearch/action/support/nodes/TransportNodesInfoActionTests.java b/server/src/test/java/org/opensearch/action/support/nodes/TransportNodesInfoActionTests.java index e9e09d0dbbbf9..8277dcd363a8d 100644 --- a/server/src/test/java/org/opensearch/action/support/nodes/TransportNodesInfoActionTests.java +++ b/server/src/test/java/org/opensearch/action/support/nodes/TransportNodesInfoActionTests.java @@ -31,32 +31,12 @@ public class TransportNodesInfoActionTests extends TransportNodesActionTests { - /** - * By default, we send discovery nodes list to each request that is sent across from the coordinator node. This - * behavior is asserted in this test. - */ - public void testNodesInfoActionWithRetentionOfDiscoveryNodesList() { - NodesInfoRequest request = new NodesInfoRequest(); - request.setIncludeDiscoveryNodes(true); - Map> combinedSentRequest = performNodesInfoAction(request); - - assertNotNull(combinedSentRequest); - combinedSentRequest.forEach((node, capturedRequestList) -> { - assertNotNull(capturedRequestList); - capturedRequestList.forEach(sentRequest -> { - assertNotNull(sentRequest.getDiscoveryNodes()); - assertEquals(sentRequest.getDiscoveryNodes().length, clusterService.state().nodes().getSize()); - }); - }); - } - /** * In the optimized ClusterStats Request, we do not send the DiscoveryNodes List to each node. This behavior is * asserted in this test. */ public void testNodesInfoActionWithoutRetentionOfDiscoveryNodesList() { NodesInfoRequest request = new NodesInfoRequest(); - request.setIncludeDiscoveryNodes(false); Map> combinedSentRequest = performNodesInfoAction(request); assertNotNull(combinedSentRequest); diff --git a/server/src/test/java/org/opensearch/action/support/nodes/TransportNodesStatsActionTests.java b/server/src/test/java/org/opensearch/action/support/nodes/TransportNodesStatsActionTests.java index c7c420e353e1a..5e74dcdbc4953 100644 --- a/server/src/test/java/org/opensearch/action/support/nodes/TransportNodesStatsActionTests.java +++ b/server/src/test/java/org/opensearch/action/support/nodes/TransportNodesStatsActionTests.java @@ -31,31 +31,11 @@ public class TransportNodesStatsActionTests extends TransportNodesActionTests { /** - * By default, we send discovery nodes list to each request that is sent across from the coordinator node. This - * behavior is asserted in this test. - */ - public void testNodesStatsActionWithRetentionOfDiscoveryNodesList() { - NodesStatsRequest request = new NodesStatsRequest(); - request.setIncludeDiscoveryNodes(true); - Map> combinedSentRequest = performNodesStatsAction(request); - - assertNotNull(combinedSentRequest); - combinedSentRequest.forEach((node, capturedRequestList) -> { - assertNotNull(capturedRequestList); - capturedRequestList.forEach(sentRequest -> { - assertNotNull(sentRequest.getDiscoveryNodes()); - assertEquals(sentRequest.getDiscoveryNodes().length, clusterService.state().nodes().getSize()); - }); - }); - } - - /** - * By default, we send discovery nodes list to each request that is sent across from the coordinator node. This - * behavior is asserted in this test. + * We don't want to send discovery nodes list to each request that is sent across from the coordinator node. + * This behavior is asserted in this test. */ public void testNodesStatsActionWithoutRetentionOfDiscoveryNodesList() { NodesStatsRequest request = new NodesStatsRequest(); - request.setIncludeDiscoveryNodes(false); Map> combinedSentRequest = performNodesStatsAction(request); assertNotNull(combinedSentRequest); From 48cf5f061094d89aaa564b97667f4965556a8e49 Mon Sep 17 00:00:00 2001 From: Lakshya Taragi <157457166+ltaragi@users.noreply.github.com> Date: Wed, 4 Sep 2024 15:00:38 +0530 Subject: [PATCH 2/8] Mute failing snapshot status tests (#15652) Signed-off-by: Lakshya Taragi --- .../test/snapshot.status/10_basic.yml | 70 ++++++++++--------- 1 file changed, 36 insertions(+), 34 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.status/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.status/10_basic.yml index c35f2419bdc91..819f04407b219 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.status/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.status/10_basic.yml @@ -25,38 +25,40 @@ setup: snapshot: test_snapshot wait_for_completion: true - - do: - snapshot.status: - repository: test_repo_status_1 - snapshot: test_snapshot - - - is_true: snapshots - - match: { snapshots.0.snapshot: test_snapshot } - - match: { snapshots.0.state: SUCCESS } - - gt: { snapshots.0.stats.incremental.file_count: 0 } - - gt: { snapshots.0.stats.incremental.size_in_bytes: 0 } - - gt: { snapshots.0.stats.total.file_count: 0 } - - gt: { snapshots.0.stats.total.size_in_bytes: 0 } - - is_true: snapshots.0.stats.start_time_in_millis -## fast in memory snapshots can take less than one millisecond to complete. - - gte: { snapshots.0.stats.time_in_millis: 0 } - ---- -"Get missing snapshot status throws an exception": - - - do: - catch: /snapshot_missing_exception.+ is missing/ - snapshot.status: - repository: test_repo_status_1 - snapshot: test_nonexistent_snapshot - ---- -"Get missing snapshot status succeeds when ignoreUnavailable is true": - - - do: - snapshot.status: - repository: test_repo_status_1 - snapshot: test_nonexistent_snapshot - ignore_unavailable: true +# TODO: fix and unmute tests - - is_true: snapshots +# - do: +# snapshot.status: +# repository: test_repo_status_1 +# snapshot: test_snapshot +# +# - is_true: snapshots +# - match: { snapshots.0.snapshot: test_snapshot } +# - match: { snapshots.0.state: SUCCESS } +# - gt: { snapshots.0.stats.incremental.file_count: 0 } +# - gt: { snapshots.0.stats.incremental.size_in_bytes: 0 } +# - gt: { snapshots.0.stats.total.file_count: 0 } +# - gt: { snapshots.0.stats.total.size_in_bytes: 0 } +# - is_true: snapshots.0.stats.start_time_in_millis +### fast in memory snapshots can take less than one millisecond to complete. +# - gte: { snapshots.0.stats.time_in_millis: 0 } +# +#--- +#"Get missing snapshot status throws an exception": +# +# - do: +# catch: /snapshot_missing_exception.+ is missing/ +# snapshot.status: +# repository: test_repo_status_1 +# snapshot: test_nonexistent_snapshot +# +#--- +#"Get missing snapshot status succeeds when ignoreUnavailable is true": +# +# - do: +# snapshot.status: +# repository: test_repo_status_1 +# snapshot: test_nonexistent_snapshot +# ignore_unavailable: true +# +# - is_true: snapshots From 23f5c2f988a7e78c3438ad2aeacd029f7a9e3ba6 Mon Sep 17 00:00:00 2001 From: Sarthak Aggarwal Date: Wed, 4 Sep 2024 15:03:56 +0530 Subject: [PATCH 3/8] [Star Tree] Scaled Float Support (#15442) --------- Signed-off-by: Sarthak Aggarwal --- .../index/mapper/ScaledFloatFieldMapper.java | 23 +++- .../mapper/ScaledFloatFieldMapperTests.java | 108 +++++++++++++++-- .../index/mapper/StarTreeMapperIT.java | 73 +++++------- .../datacube/DimensionFactory.java | 12 +- .../datacube/DimensionType.java | 31 +++++ .../aggregators/CountValueAggregator.java | 9 +- .../aggregators/DocCountAggregator.java | 7 +- .../aggregators/MaxValueAggregator.java | 6 +- .../aggregators/MetricAggregatorInfo.java | 15 ++- .../aggregators/MinValueAggregator.java | 6 +- .../StatelessDoubleValueAggregator.java | 17 +-- .../aggregators/SumValueAggregator.java | 19 +-- .../startree/aggregators/ValueAggregator.java | 4 +- .../aggregators/ValueAggregatorFactory.java | 12 +- .../numerictype/StarTreeNumericType.java | 66 ----------- .../StarTreeNumericTypeConverters.java | 58 --------- .../aggregators/numerictype/package-info.java | 14 --- .../builder/AbstractDocumentsFileManager.java | 67 ++++++----- .../startree/builder/BaseStarTreeBuilder.java | 41 ++++--- .../node/FixedLengthStarTreeNode.java | 2 +- .../index/mapper/DateFieldMapper.java | 1 + .../index/mapper/DocCountFieldMapper.java | 5 + .../index/mapper/FieldValueConverter.java | 30 +++++ .../org/opensearch/index/mapper/Mapper.java | 2 +- .../index/mapper/MapperBuilderProperties.java | 41 +++++++ .../index/mapper/NumberFieldMapper.java | 77 +++++++++++- .../index/mapper/StarTreeMapper.java | 9 +- .../StarTreeDocValuesFormatTests.java | 4 +- .../datacube/startree/StarTreeTestUtils.java | 14 +-- .../AbstractValueAggregatorTests.java | 23 ++-- .../CountValueAggregatorTests.java | 8 +- .../aggregators/DocCountAggregatorTests.java | 10 +- .../aggregators/MaxValueAggregatorTests.java | 19 +-- .../MetricAggregatorInfoTests.java | 29 +++-- .../aggregators/MinValueAggregatorTests.java | 19 +-- .../StaticValueAggregatorTests.java | 12 +- .../aggregators/SumValueAggregatorTests.java | 16 +-- .../ValueAggregatorFactoryTests.java | 12 +- .../builder/AbstractStarTreeBuilderTests.java | 8 +- .../builder/BaseStarTreeBuilderTests.java | 5 +- .../meta/StarTreeMetadataTests.java | 4 +- .../index/mapper/ObjectMapperTests.java | 10 +- .../index/mapper/StarTreeMapperTests.java | 110 ++++++------------ 43 files changed, 575 insertions(+), 483 deletions(-) create mode 100644 server/src/main/java/org/opensearch/index/compositeindex/datacube/DimensionType.java delete mode 100644 server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/numerictype/StarTreeNumericType.java delete mode 100644 server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/numerictype/StarTreeNumericTypeConverters.java delete mode 100644 server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/numerictype/package-info.java create mode 100644 server/src/main/java/org/opensearch/index/mapper/FieldValueConverter.java create mode 100644 server/src/main/java/org/opensearch/index/mapper/MapperBuilderProperties.java diff --git a/modules/mapper-extras/src/main/java/org/opensearch/index/mapper/ScaledFloatFieldMapper.java b/modules/mapper-extras/src/main/java/org/opensearch/index/mapper/ScaledFloatFieldMapper.java index 3115dce6c10a5..b46b58f415cfd 100644 --- a/modules/mapper-extras/src/main/java/org/opensearch/index/mapper/ScaledFloatFieldMapper.java +++ b/modules/mapper-extras/src/main/java/org/opensearch/index/mapper/ScaledFloatFieldMapper.java @@ -49,6 +49,7 @@ import org.opensearch.common.xcontent.support.XContentMapValues; import org.opensearch.core.xcontent.XContentParser; import org.opensearch.core.xcontent.XContentParser.Token; +import org.opensearch.index.compositeindex.datacube.DimensionType; import org.opensearch.index.fielddata.FieldData; import org.opensearch.index.fielddata.IndexFieldData; import org.opensearch.index.fielddata.IndexNumericFieldData; @@ -71,10 +72,12 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.function.Supplier; /** A {@link FieldMapper} for scaled floats. Values are internally multiplied - * by a scaling factor and rounded to the closest long. */ + * by a scaling factor and rounded to the closest long. + */ public class ScaledFloatFieldMapper extends ParametrizedFieldMapper { public static final String CONTENT_TYPE = "scaled_float"; @@ -162,11 +165,21 @@ public ScaledFloatFieldMapper build(BuilderContext context) { ); return new ScaledFloatFieldMapper(name, type, multiFieldsBuilder.build(this, context), copyTo.build(), this); } + + @Override + public Optional getSupportedDataCubeDimensionType() { + return Optional.of(DimensionType.NUMERIC); + } + + @Override + public boolean isDataCubeMetricSupported() { + return true; + } } public static final TypeParser PARSER = new TypeParser((n, c) -> new Builder(n, c.getSettings())); - public static final class ScaledFloatFieldType extends SimpleMappedFieldType implements NumericPointEncoder { + public static final class ScaledFloatFieldType extends SimpleMappedFieldType implements NumericPointEncoder, FieldValueConverter { private final double scalingFactor; private final Double nullValue; @@ -340,6 +353,12 @@ public DocValueFormat docValueFormat(String format, ZoneId timeZone) { private double scale(Object input) { return new BigDecimal(Double.toString(parse(input))).multiply(BigDecimal.valueOf(scalingFactor)).doubleValue(); } + + @Override + public double toDoubleValue(long value) { + double inverseScalingFactor = 1d / scalingFactor; + return value * inverseScalingFactor; + } } private final Explicit ignoreMalformed; diff --git a/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/ScaledFloatFieldMapperTests.java b/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/ScaledFloatFieldMapperTests.java index 45507867f6691..c3d62b088ced7 100644 --- a/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/ScaledFloatFieldMapperTests.java +++ b/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/ScaledFloatFieldMapperTests.java @@ -34,11 +34,16 @@ import org.apache.lucene.index.DocValuesType; import org.apache.lucene.index.IndexableField; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.xcontent.MediaTypeRegistry; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings; import org.opensearch.plugins.Plugin; +import org.junit.AfterClass; +import org.junit.BeforeClass; import java.io.IOException; import java.util.Arrays; @@ -46,6 +51,7 @@ import java.util.List; import static java.util.Collections.singletonList; +import static org.opensearch.common.util.FeatureFlags.STAR_TREE_INDEX; import static org.hamcrest.Matchers.containsString; public class ScaledFloatFieldMapperTests extends MapperTestCase { @@ -91,24 +97,112 @@ public void testExistsQueryDocValuesDisabled() throws IOException { assertParseMinimalWarnings(); } - public void testDefaults() throws Exception { - XContentBuilder mapping = fieldMapping(b -> b.field("type", "scaled_float").field("scaling_factor", 10.0)); + @BeforeClass + public static void createMapper() { + FeatureFlags.initializeFeatureFlags(Settings.builder().put(STAR_TREE_INDEX, "true").build()); + } + + @AfterClass + public static void clearMapper() { + FeatureFlags.initializeFeatureFlags(Settings.EMPTY); + } + + public void testScaledFloatWithStarTree() throws Exception { + + double scalingFactorField1 = randomDouble() * 100; + double scalingFactorField2 = randomDouble() * 100; + double scalingFactorField3 = randomDouble() * 100; + + XContentBuilder mapping = getStarTreeMappingWithScaledFloat(scalingFactorField1, scalingFactorField2, scalingFactorField3); DocumentMapper mapper = createDocumentMapper(mapping); - assertEquals(mapping.toString(), mapper.mappingSource().toString()); + assertTrue(mapping.toString().contains("startree")); - ParsedDocument doc = mapper.parse(source(b -> b.field("field", 123))); - IndexableField[] fields = doc.rootDoc().getFields("field"); + long randomLongField1 = randomLong(); + long randomLongField2 = randomLong(); + long randomLongField3 = randomLong(); + ParsedDocument doc = mapper.parse( + source(b -> b.field("field1", randomLongField1).field("field2", randomLongField2).field("field3", randomLongField3)) + ); + validateScaledFloatFields(doc, "field1", randomLongField1, scalingFactorField1); + validateScaledFloatFields(doc, "field2", randomLongField2, scalingFactorField2); + validateScaledFloatFields(doc, "field3", randomLongField3, scalingFactorField3); + } + + @Override + protected Settings getIndexSettings() { + return Settings.builder() + .put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), true) + .put(super.getIndexSettings()) + .build(); + } + + private static void validateScaledFloatFields(ParsedDocument doc, String field, long value, double scalingFactor) { + IndexableField[] fields = doc.rootDoc().getFields(field); assertEquals(2, fields.length); IndexableField pointField = fields[0]; assertEquals(1, pointField.fieldType().pointDimensionCount()); assertFalse(pointField.fieldType().stored()); - assertEquals(1230, pointField.numericValue().longValue()); + assertEquals((long) (value * scalingFactor), pointField.numericValue().longValue()); IndexableField dvField = fields[1]; assertEquals(DocValuesType.SORTED_NUMERIC, dvField.fieldType().docValuesType()); - assertEquals(1230, dvField.numericValue().longValue()); + assertEquals((long) (value * scalingFactor), dvField.numericValue().longValue()); assertFalse(dvField.fieldType().stored()); } + private XContentBuilder getStarTreeMappingWithScaledFloat( + double scalingFactorField1, + double scalingFactorField2, + double scalingFactorField3 + ) throws IOException { + return topMapping(b -> { + b.startObject("composite"); + b.startObject("startree"); + b.field("type", "star_tree"); + b.startObject("config"); + b.field("max_leaf_docs", 100); + b.startArray("ordered_dimensions"); + b.startObject(); + b.field("name", "field1"); + b.endObject(); + b.startObject(); + b.field("name", "field2"); + b.endObject(); + b.endArray(); + b.startArray("metrics"); + b.startObject(); + b.field("name", "field3"); + b.startArray("stats"); + b.value("sum"); + b.value("value_count"); + b.endArray(); + b.endObject(); + b.endArray(); + b.endObject(); + b.endObject(); + b.endObject(); + b.startObject("properties"); + b.startObject("field1"); + b.field("type", "scaled_float").field("scaling_factor", scalingFactorField1); + b.endObject(); + b.startObject("field2"); + b.field("type", "scaled_float").field("scaling_factor", scalingFactorField2); + b.endObject(); + b.startObject("field3"); + b.field("type", "scaled_float").field("scaling_factor", scalingFactorField3); + b.endObject(); + b.endObject(); + }); + } + + public void testDefaults() throws Exception { + XContentBuilder mapping = fieldMapping(b -> b.field("type", "scaled_float").field("scaling_factor", 10.0)); + DocumentMapper mapper = createDocumentMapper(mapping); + assertEquals(mapping.toString(), mapper.mappingSource().toString()); + + ParsedDocument doc = mapper.parse(source(b -> b.field("field", 123))); + validateScaledFloatFields(doc, "field", 123, 10.0); + } + public void testMissingScalingFactor() { Exception e = expectThrows( MapperParsingException.class, diff --git a/server/src/internalClusterTest/java/org/opensearch/index/mapper/StarTreeMapperIT.java b/server/src/internalClusterTest/java/org/opensearch/index/mapper/StarTreeMapperIT.java index 8cfb710679137..0d69b762ab4f2 100644 --- a/server/src/internalClusterTest/java/org/opensearch/index/mapper/StarTreeMapperIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/index/mapper/StarTreeMapperIT.java @@ -12,7 +12,6 @@ import org.opensearch.action.index.IndexResponse; import org.opensearch.action.search.SearchResponse; import org.opensearch.action.support.master.AcknowledgedResponse; -import org.opensearch.common.Rounding; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; import org.opensearch.core.common.unit.ByteSizeUnit; @@ -23,7 +22,6 @@ import org.opensearch.index.IndexService; import org.opensearch.index.IndexSettings; import org.opensearch.index.compositeindex.CompositeIndexSettings; -import org.opensearch.index.compositeindex.datacube.DateDimension; import org.opensearch.index.compositeindex.datacube.MetricStat; import org.opensearch.index.compositeindex.datacube.startree.StarTreeFieldConfiguration; import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings; @@ -62,7 +60,10 @@ private static XContentBuilder createMinimalTestMapping(boolean invalidDim, bool .startObject("config") .startArray("ordered_dimensions") .startObject() - .field("name", "timestamp") + .field("name", "numeric_dv_1") + .endObject() + .startObject() + .field("name", "numeric_dv_2") .endObject() .startObject() .field("name", getDim(invalidDim, keywordDim)) @@ -84,6 +85,14 @@ private static XContentBuilder createMinimalTestMapping(boolean invalidDim, bool .field("type", "integer") .field("doc_values", true) .endObject() + .startObject("numeric_dv_1") + .field("type", "integer") + .field("doc_values", true) + .endObject() + .startObject("numeric_dv_2") + .field("type", "integer") + .field("doc_values", true) + .endObject() .startObject("numeric") .field("type", "integer") .field("doc_values", false) @@ -112,11 +121,7 @@ private static XContentBuilder createMaxDimTestMapping() { .startObject("config") .startArray("ordered_dimensions") .startObject() - .field("name", "timestamp") - .startArray("calendar_intervals") - .value("day") - .value("month") - .endArray() + .field("name", "dim4") .endObject() .startObject() .field("name", "dim2") @@ -201,7 +206,7 @@ private static XContentBuilder createUpdateTestMapping(boolean changeDim, boolea .startObject("config") .startArray("ordered_dimensions") .startObject() - .field("name", "timestamp") + .field("name", "numeric_dv1") .endObject() .startObject() .field("name", changeDim ? "numeric_new" : getDim(false, false)) @@ -223,6 +228,10 @@ private static XContentBuilder createUpdateTestMapping(boolean changeDim, boolea .field("type", "integer") .field("doc_values", true) .endObject() + .startObject("numeric_dv1") + .field("type", "integer") + .field("doc_values", true) + .endObject() .startObject("numeric") .field("type", "integer") .field("doc_values", false) @@ -256,7 +265,7 @@ private XContentBuilder getMappingWithDuplicateFields(boolean isDuplicateDim, bo .startObject("config") .startArray("ordered_dimensions") .startObject() - .field("name", "timestamp") + .field("name", "numeric_dv2") .endObject() .startObject() .field("name", "numeric_dv") @@ -284,6 +293,10 @@ private XContentBuilder getMappingWithDuplicateFields(boolean isDuplicateDim, bo .field("type", "integer") .field("doc_values", true) .endObject() + .startObject("numeric_dv2") + .field("type", "integer") + .field("doc_values", true) + .endObject() .startObject("numeric_dv1") .field("type", "integer") .field("doc_values", true) @@ -328,15 +341,8 @@ public void testValidCompositeIndex() { for (CompositeMappedFieldType ft : fts) { assertTrue(ft instanceof StarTreeMapper.StarTreeFieldType); StarTreeMapper.StarTreeFieldType starTreeFieldType = (StarTreeMapper.StarTreeFieldType) ft; - assertEquals("timestamp", starTreeFieldType.getDimensions().get(0).getField()); - assertTrue(starTreeFieldType.getDimensions().get(0) instanceof DateDimension); - DateDimension dateDim = (DateDimension) starTreeFieldType.getDimensions().get(0); - List expectedTimeUnits = Arrays.asList( - Rounding.DateTimeUnit.MINUTES_OF_HOUR, - Rounding.DateTimeUnit.HOUR_OF_DAY - ); - assertEquals(expectedTimeUnits, dateDim.getIntervals()); - assertEquals("numeric_dv", starTreeFieldType.getDimensions().get(1).getField()); + assertEquals("numeric_dv_1", starTreeFieldType.getDimensions().get(0).getField()); + assertEquals("numeric_dv_2", starTreeFieldType.getDimensions().get(1).getField()); assertEquals(2, starTreeFieldType.getMetrics().size()); assertEquals("numeric_dv", starTreeFieldType.getMetrics().get(0).getField()); @@ -496,15 +502,8 @@ public void testUpdateIndexWhenMappingIsSame() { for (CompositeMappedFieldType ft : fts) { assertTrue(ft instanceof StarTreeMapper.StarTreeFieldType); StarTreeMapper.StarTreeFieldType starTreeFieldType = (StarTreeMapper.StarTreeFieldType) ft; - assertEquals("timestamp", starTreeFieldType.getDimensions().get(0).getField()); - assertTrue(starTreeFieldType.getDimensions().get(0) instanceof DateDimension); - DateDimension dateDim = (DateDimension) starTreeFieldType.getDimensions().get(0); - List expectedTimeUnits = Arrays.asList( - Rounding.DateTimeUnit.MINUTES_OF_HOUR, - Rounding.DateTimeUnit.HOUR_OF_DAY - ); - assertEquals(expectedTimeUnits, dateDim.getIntervals()); - assertEquals("numeric_dv", starTreeFieldType.getDimensions().get(1).getField()); + assertEquals("numeric_dv_1", starTreeFieldType.getDimensions().get(0).getField()); + assertEquals("numeric_dv_2", starTreeFieldType.getDimensions().get(1).getField()); assertEquals("numeric_dv", starTreeFieldType.getMetrics().get(0).getField()); // Assert default metrics @@ -570,24 +569,6 @@ public void testMaxMetricsCompositeIndex() { ); } - public void testMaxCalendarIntervalsCompositeIndex() { - MapperParsingException ex = expectThrows( - MapperParsingException.class, - () -> prepareCreate(TEST_INDEX).setMapping(createMaxDimTestMapping()) - .setSettings( - Settings.builder() - .put(StarTreeIndexSettings.STAR_TREE_MAX_DATE_INTERVALS_SETTING.getKey(), 1) - .put(StarTreeIndexSettings.IS_COMPOSITE_INDEX_SETTING.getKey(), true) - .put(IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING.getKey(), new ByteSizeValue(512, ByteSizeUnit.MB)) - ) - .get() - ); - assertEquals( - "Failed to parse mapping [_doc]: At most [1] calendar intervals are allowed in dimension [timestamp]", - ex.getMessage() - ); - } - public void testUnsupportedDim() { MapperParsingException ex = expectThrows( MapperParsingException.class, diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/DimensionFactory.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/DimensionFactory.java index 6a09e947217f5..3c418c68fe8ad 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/datacube/DimensionFactory.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/DimensionFactory.java @@ -12,9 +12,7 @@ import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.common.xcontent.support.XContentMapValues; import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings; -import org.opensearch.index.mapper.DateFieldMapper; import org.opensearch.index.mapper.Mapper; -import org.opensearch.index.mapper.NumberFieldMapper; import java.util.ArrayList; import java.util.List; @@ -55,11 +53,13 @@ public static Dimension parseAndCreateDimension( Map dimensionMap, Mapper.TypeParser.ParserContext c ) { - if (builder instanceof DateFieldMapper.Builder) { + if (builder.getSupportedDataCubeDimensionType().isPresent() + && builder.getSupportedDataCubeDimensionType().get().equals(DimensionType.DATE)) { return parseAndCreateDateDimension(name, dimensionMap, c); - } else if (builder instanceof NumberFieldMapper.Builder) { - return new NumericDimension(name); - } + } else if (builder.getSupportedDataCubeDimensionType().isPresent() + && builder.getSupportedDataCubeDimensionType().get().equals(DimensionType.NUMERIC)) { + return new NumericDimension(name); + } throw new IllegalArgumentException( String.format(Locale.ROOT, "unsupported field type associated with star tree dimension [%s]", name) ); diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/DimensionType.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/DimensionType.java new file mode 100644 index 0000000000000..4b9faea331752 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/DimensionType.java @@ -0,0 +1,31 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.index.compositeindex.datacube; + +/** + * Represents the types of dimensions supported in a data cube. + *

+ * This enum defines the possible types of dimensions that can be used + * in a data cube structure within the composite index. + * + * @opensearch.experimental + */ +public enum DimensionType { + /** + * Represents a numeric dimension type. + * This is used for dimensions that contain numerical values. + */ + NUMERIC, + + /** + * Represents a date dimension type. + * This is used for dimensions that contain date or timestamp values. + */ + DATE +} diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/CountValueAggregator.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/CountValueAggregator.java index e79abe0f170b3..2bdbab1cd5b81 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/CountValueAggregator.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/CountValueAggregator.java @@ -7,22 +7,23 @@ */ package org.opensearch.index.compositeindex.datacube.startree.aggregators; -import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType; +import org.opensearch.index.mapper.FieldValueConverter; +import org.opensearch.index.mapper.NumberFieldMapper; /** * Count value aggregator for star tree * * @opensearch.experimental */ -class CountValueAggregator implements ValueAggregator { +public class CountValueAggregator implements ValueAggregator { public static final long DEFAULT_INITIAL_VALUE = 1L; - private static final StarTreeNumericType VALUE_AGGREGATOR_TYPE = StarTreeNumericType.LONG; + private static final FieldValueConverter VALUE_AGGREGATOR_TYPE = NumberFieldMapper.NumberType.LONG; public CountValueAggregator() {} @Override - public StarTreeNumericType getAggregatedValueType() { + public FieldValueConverter getAggregatedValueType() { return VALUE_AGGREGATOR_TYPE; } diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/DocCountAggregator.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/DocCountAggregator.java index 0896fa54e9f46..0f354aad063c2 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/DocCountAggregator.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/DocCountAggregator.java @@ -8,7 +8,8 @@ package org.opensearch.index.compositeindex.datacube.startree.aggregators; -import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType; +import org.opensearch.index.mapper.FieldValueConverter; +import org.opensearch.index.mapper.NumberFieldMapper; /** * Aggregator to handle '_doc_count' field @@ -17,12 +18,12 @@ */ public class DocCountAggregator implements ValueAggregator { - private static final StarTreeNumericType VALUE_AGGREGATOR_TYPE = StarTreeNumericType.LONG; + private static final FieldValueConverter VALUE_AGGREGATOR_TYPE = NumberFieldMapper.NumberType.LONG; public DocCountAggregator() {} @Override - public StarTreeNumericType getAggregatedValueType() { + public FieldValueConverter getAggregatedValueType() { return VALUE_AGGREGATOR_TYPE; } diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MaxValueAggregator.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MaxValueAggregator.java index 8e7bb44238e40..34a3bdc32226d 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MaxValueAggregator.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MaxValueAggregator.java @@ -7,7 +7,7 @@ */ package org.opensearch.index.compositeindex.datacube.startree.aggregators; -import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType; +import org.opensearch.index.mapper.FieldValueConverter; /** * Max value aggregator for star tree @@ -16,8 +16,8 @@ */ class MaxValueAggregator extends StatelessDoubleValueAggregator { - public MaxValueAggregator(StarTreeNumericType starTreeNumericType) { - super(starTreeNumericType, null); + public MaxValueAggregator(FieldValueConverter fieldValueConverter) { + super(fieldValueConverter, null); } @Override diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MetricAggregatorInfo.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MetricAggregatorInfo.java index 8b6db2a183bf8..487e714c9e44e 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MetricAggregatorInfo.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MetricAggregatorInfo.java @@ -8,8 +8,7 @@ package org.opensearch.index.compositeindex.datacube.startree.aggregators; import org.opensearch.index.compositeindex.datacube.MetricStat; -import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType; -import org.opensearch.index.fielddata.IndexNumericFieldData; +import org.opensearch.index.mapper.FieldValueConverter; import java.util.Comparator; import java.util.Objects; @@ -27,15 +26,15 @@ public class MetricAggregatorInfo implements Comparable { private final MetricStat metricStat; private final String field; private final ValueAggregator valueAggregators; - private final StarTreeNumericType starTreeNumericType; + private final FieldValueConverter fieldValueConverter; /** * Constructor for MetricAggregatorInfo */ - public MetricAggregatorInfo(MetricStat metricStat, String field, String starFieldName, IndexNumericFieldData.NumericType numericType) { + public MetricAggregatorInfo(MetricStat metricStat, String field, String starFieldName, FieldValueConverter fieldValueConverter) { this.metricStat = metricStat; - this.starTreeNumericType = StarTreeNumericType.fromNumericType(numericType); - this.valueAggregators = ValueAggregatorFactory.getValueAggregator(metricStat, this.starTreeNumericType); + this.fieldValueConverter = fieldValueConverter; + this.valueAggregators = ValueAggregatorFactory.getValueAggregator(metricStat, this.fieldValueConverter); this.field = field; this.starFieldName = starFieldName; this.metric = toFieldName(); @@ -72,8 +71,8 @@ public ValueAggregator getValueAggregators() { /** * @return star tree aggregated value type */ - public StarTreeNumericType getAggregatedValueType() { - return starTreeNumericType; + public FieldValueConverter getNumericFieldConverter() { + return fieldValueConverter; } /** diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MinValueAggregator.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MinValueAggregator.java index 46e9188b5dc2f..b085447be758e 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MinValueAggregator.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MinValueAggregator.java @@ -7,7 +7,7 @@ */ package org.opensearch.index.compositeindex.datacube.startree.aggregators; -import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType; +import org.opensearch.index.mapper.FieldValueConverter; /** * Min value aggregator for star tree @@ -16,8 +16,8 @@ */ class MinValueAggregator extends StatelessDoubleValueAggregator { - public MinValueAggregator(StarTreeNumericType starTreeNumericType) { - super(starTreeNumericType, null); + public MinValueAggregator(FieldValueConverter fieldValueConverter) { + super(fieldValueConverter, null); } @Override diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/StatelessDoubleValueAggregator.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/StatelessDoubleValueAggregator.java index 30a1c47c0ee9b..d83a828fb0f9c 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/StatelessDoubleValueAggregator.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/StatelessDoubleValueAggregator.java @@ -7,7 +7,8 @@ */ package org.opensearch.index.compositeindex.datacube.startree.aggregators; -import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType; +import org.opensearch.index.mapper.FieldValueConverter; +import org.opensearch.index.mapper.NumberFieldMapper; /** * This is an abstract class that defines the common methods for all double value aggregators @@ -17,17 +18,17 @@ */ abstract class StatelessDoubleValueAggregator implements ValueAggregator { - protected final StarTreeNumericType starTreeNumericType; + protected final FieldValueConverter fieldValueConverter; protected final Double identityValue; - private static final StarTreeNumericType VALUE_AGGREGATOR_TYPE = StarTreeNumericType.DOUBLE; + private static final FieldValueConverter VALUE_AGGREGATOR_TYPE = NumberFieldMapper.NumberType.DOUBLE; - public StatelessDoubleValueAggregator(StarTreeNumericType starTreeNumericType, Double identityValue) { - this.starTreeNumericType = starTreeNumericType; + public StatelessDoubleValueAggregator(FieldValueConverter fieldValueConverter, Double identityValue) { + this.fieldValueConverter = fieldValueConverter; this.identityValue = identityValue; } @Override - public StarTreeNumericType getAggregatedValueType() { + public FieldValueConverter getAggregatedValueType() { return VALUE_AGGREGATOR_TYPE; } @@ -36,7 +37,7 @@ public Double getInitialAggregatedValueForSegmentDocValue(Long segmentDocValue) if (segmentDocValue == null) { return getIdentityMetricValue(); } - return starTreeNumericType.getDoubleValue(segmentDocValue); + return fieldValueConverter.toDoubleValue(segmentDocValue); } @Override @@ -57,7 +58,7 @@ public Double toAggregatedValueType(Long value) { if (value == null) { return getIdentityMetricValue(); } - return VALUE_AGGREGATOR_TYPE.getDoubleValue(value); + return VALUE_AGGREGATOR_TYPE.toDoubleValue(value); } catch (Exception e) { throw new IllegalStateException("Cannot convert " + value + " to sortable aggregation type", e); } diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/SumValueAggregator.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/SumValueAggregator.java index ef97a9b603df3..81eb3558915be 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/SumValueAggregator.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/SumValueAggregator.java @@ -7,7 +7,8 @@ */ package org.opensearch.index.compositeindex.datacube.startree.aggregators; -import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType; +import org.opensearch.index.mapper.FieldValueConverter; +import org.opensearch.index.mapper.NumberFieldMapper; import org.opensearch.search.aggregations.metrics.CompensatedSum; /** @@ -22,17 +23,17 @@ */ class SumValueAggregator implements ValueAggregator { - private final StarTreeNumericType starTreeNumericType; - private static final StarTreeNumericType VALUE_AGGREGATOR_TYPE = StarTreeNumericType.DOUBLE; + private final FieldValueConverter fieldValueConverter; + private static final FieldValueConverter VALUE_AGGREGATOR_TYPE = NumberFieldMapper.NumberType.DOUBLE; private CompensatedSum kahanSummation = new CompensatedSum(0, 0); - public SumValueAggregator(StarTreeNumericType starTreeNumericType) { - this.starTreeNumericType = starTreeNumericType; + public SumValueAggregator(FieldValueConverter fieldValueConverter) { + this.fieldValueConverter = fieldValueConverter; } @Override - public StarTreeNumericType getAggregatedValueType() { + public FieldValueConverter getAggregatedValueType() { return VALUE_AGGREGATOR_TYPE; } @@ -41,7 +42,7 @@ public Double getInitialAggregatedValueForSegmentDocValue(Long segmentDocValue) kahanSummation.reset(0, 0); // add takes care of the sum and compensation internally if (segmentDocValue != null) { - kahanSummation.add(starTreeNumericType.getDoubleValue(segmentDocValue)); + kahanSummation.add(fieldValueConverter.toDoubleValue(segmentDocValue)); } else { kahanSummation.add(getIdentityMetricValue()); } @@ -55,7 +56,7 @@ public Double mergeAggregatedValueAndSegmentValue(Double value, Long segmentDocV assert value == null || kahanSummation.value() == value; // add takes care of the sum and compensation internally if (segmentDocValue != null) { - kahanSummation.add(starTreeNumericType.getDoubleValue(segmentDocValue)); + kahanSummation.add(fieldValueConverter.toDoubleValue(segmentDocValue)); } else { kahanSummation.add(getIdentityMetricValue()); } @@ -92,7 +93,7 @@ public Double toAggregatedValueType(Long value) { if (value == null) { return getIdentityMetricValue(); } - return VALUE_AGGREGATOR_TYPE.getDoubleValue(value); + return VALUE_AGGREGATOR_TYPE.toDoubleValue(value); } catch (Exception e) { throw new IllegalStateException("Cannot convert " + value + " to sortable aggregation type", e); } diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/ValueAggregator.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/ValueAggregator.java index d5ca7f3493087..8eac45669c063 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/ValueAggregator.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/ValueAggregator.java @@ -7,7 +7,7 @@ */ package org.opensearch.index.compositeindex.datacube.startree.aggregators; -import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType; +import org.opensearch.index.mapper.FieldValueConverter; /** * A value aggregator that pre-aggregates on the input values for a specific type of aggregation. @@ -19,7 +19,7 @@ public interface ValueAggregator { /** * Returns the data type of the aggregated value. */ - StarTreeNumericType getAggregatedValueType(); + FieldValueConverter getAggregatedValueType(); /** * Returns the initial aggregated value. diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/ValueAggregatorFactory.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/ValueAggregatorFactory.java index bdc381110365d..aac5d2b2855d3 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/ValueAggregatorFactory.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/ValueAggregatorFactory.java @@ -8,7 +8,7 @@ package org.opensearch.index.compositeindex.datacube.startree.aggregators; import org.opensearch.index.compositeindex.datacube.MetricStat; -import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType; +import org.opensearch.index.mapper.FieldValueConverter; /** * Value aggregator factory for a given aggregation type @@ -22,20 +22,20 @@ private ValueAggregatorFactory() {} * Returns a new instance of value aggregator for the given aggregation type. * * @param aggregationType Aggregation type - * @param starTreeNumericType Numeric type associated with star tree field ( as specified in index mapping ) + * @param fieldValueConverter Numeric type converter associated with star tree field ( as specified in index mapping ) * @return Value aggregator */ - public static ValueAggregator getValueAggregator(MetricStat aggregationType, StarTreeNumericType starTreeNumericType) { + public static ValueAggregator getValueAggregator(MetricStat aggregationType, FieldValueConverter fieldValueConverter) { switch (aggregationType) { // avg aggregator will be covered in the part of query (using count and sum) case SUM: - return new SumValueAggregator(starTreeNumericType); + return new SumValueAggregator(fieldValueConverter); case VALUE_COUNT: return new CountValueAggregator(); case MIN: - return new MinValueAggregator(starTreeNumericType); + return new MinValueAggregator(fieldValueConverter); case MAX: - return new MaxValueAggregator(starTreeNumericType); + return new MaxValueAggregator(fieldValueConverter); case DOC_COUNT: return new DocCountAggregator(); default: diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/numerictype/StarTreeNumericType.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/numerictype/StarTreeNumericType.java deleted file mode 100644 index 57fe573a6a93c..0000000000000 --- a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/numerictype/StarTreeNumericType.java +++ /dev/null @@ -1,66 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype; - -import org.opensearch.index.fielddata.IndexNumericFieldData; - -import java.util.function.Function; - -/** - * Enum to map Star Tree Numeric Types to Lucene's Numeric Type - * - * @opensearch.experimental - */ -public enum StarTreeNumericType { - - // TODO: Handle scaled floats - HALF_FLOAT(IndexNumericFieldData.NumericType.HALF_FLOAT, StarTreeNumericTypeConverters::halfFloatPointToDouble), - FLOAT(IndexNumericFieldData.NumericType.FLOAT, StarTreeNumericTypeConverters::floatPointToDouble), - LONG(IndexNumericFieldData.NumericType.LONG, StarTreeNumericTypeConverters::longToDouble), - DOUBLE(IndexNumericFieldData.NumericType.DOUBLE, StarTreeNumericTypeConverters::sortableLongtoDouble), - INT(IndexNumericFieldData.NumericType.INT, StarTreeNumericTypeConverters::intToDouble), - SHORT(IndexNumericFieldData.NumericType.SHORT, StarTreeNumericTypeConverters::shortToDouble), - BYTE(IndexNumericFieldData.NumericType.BYTE, StarTreeNumericTypeConverters::bytesToDouble), - UNSIGNED_LONG(IndexNumericFieldData.NumericType.UNSIGNED_LONG, StarTreeNumericTypeConverters::unsignedlongToDouble); - - final IndexNumericFieldData.NumericType numericType; - final Function converter; - - StarTreeNumericType(IndexNumericFieldData.NumericType numericType, Function converter) { - this.numericType = numericType; - this.converter = converter; - } - - public double getDoubleValue(long rawValue) { - return this.converter.apply(rawValue); - } - - public static StarTreeNumericType fromNumericType(IndexNumericFieldData.NumericType numericType) { - switch (numericType) { - case HALF_FLOAT: - return StarTreeNumericType.HALF_FLOAT; - case FLOAT: - return StarTreeNumericType.FLOAT; - case LONG: - return StarTreeNumericType.LONG; - case DOUBLE: - return StarTreeNumericType.DOUBLE; - case INT: - return StarTreeNumericType.INT; - case SHORT: - return StarTreeNumericType.SHORT; - case UNSIGNED_LONG: - return StarTreeNumericType.UNSIGNED_LONG; - case BYTE: - return StarTreeNumericType.BYTE; - default: - throw new UnsupportedOperationException("Unknown numeric type [" + numericType + "]"); - } - } -} diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/numerictype/StarTreeNumericTypeConverters.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/numerictype/StarTreeNumericTypeConverters.java deleted file mode 100644 index eb7647c4f9851..0000000000000 --- a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/numerictype/StarTreeNumericTypeConverters.java +++ /dev/null @@ -1,58 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype; - -import org.apache.lucene.sandbox.document.HalfFloatPoint; -import org.apache.lucene.util.NumericUtils; -import org.opensearch.common.Numbers; -import org.opensearch.common.annotation.ExperimentalApi; - -/** - * Numeric converters used during aggregations of metric values - * - * @opensearch.experimental - */ -@ExperimentalApi -public class StarTreeNumericTypeConverters { - - public static double halfFloatPointToDouble(Long value) { - return HalfFloatPoint.sortableShortToHalfFloat((short) value.longValue()); - } - - public static double floatPointToDouble(Long value) { - return NumericUtils.sortableIntToFloat((int) value.longValue()); - } - - public static double longToDouble(Long value) { - return (double) value; - } - - public static double intToDouble(Long value) { - return (double) value; - } - - public static double shortToDouble(Long value) { - return (double) value; - } - - public static Double sortableLongtoDouble(Long value) { - return NumericUtils.sortableLongToDouble(value); - } - - public static double unsignedlongToDouble(Long value) { - return Numbers.unsignedLongToDouble(value); - } - - public static double bytesToDouble(Long value) { - byte[] bytes = new byte[8]; - NumericUtils.longToSortableBytes(value, bytes, 0); - return NumericUtils.sortableLongToDouble(NumericUtils.sortableBytesToLong(bytes, 0)); - } - -} diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/numerictype/package-info.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/numerictype/package-info.java deleted file mode 100644 index fe5c2a7ceb254..0000000000000 --- a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/numerictype/package-info.java +++ /dev/null @@ -1,14 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -/** - * Numeric Types for Composite Index Star Tree - * - * @opensearch.experimental - */ -package org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype; diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/builder/AbstractDocumentsFileManager.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/builder/AbstractDocumentsFileManager.java index 4214a46b2fc1c..327fd26c00608 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/builder/AbstractDocumentsFileManager.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/builder/AbstractDocumentsFileManager.java @@ -19,13 +19,16 @@ import org.opensearch.index.compositeindex.datacube.startree.StarTreeDocument; import org.opensearch.index.compositeindex.datacube.startree.StarTreeField; import org.opensearch.index.compositeindex.datacube.startree.aggregators.MetricAggregatorInfo; -import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericTypeConverters; import org.opensearch.index.compositeindex.datacube.startree.utils.StarTreeDocumentBitSetUtil; +import org.opensearch.index.mapper.FieldValueConverter; import java.io.Closeable; import java.io.IOException; import java.util.List; +import static org.opensearch.index.mapper.NumberFieldMapper.NumberType.DOUBLE; +import static org.opensearch.index.mapper.NumberFieldMapper.NumberType.LONG; + /** * Abstract class for managing star tree file operations. * @@ -89,24 +92,22 @@ protected int writeDimensions(StarTreeDocument starTreeDocument, IndexOutput out protected int writeMetrics(StarTreeDocument starTreeDocument, IndexOutput output, boolean isAggregatedDoc) throws IOException { int numBytes = 0; for (int i = 0; i < starTreeDocument.metrics.length; i++) { - switch (metricAggregatorInfos.get(i).getValueAggregators().getAggregatedValueType()) { - case LONG: + FieldValueConverter aggregatedValueType = metricAggregatorInfos.get(i).getValueAggregators().getAggregatedValueType(); + if (aggregatedValueType.equals(LONG)) { + output.writeLong(starTreeDocument.metrics[i] == null ? 0L : (Long) starTreeDocument.metrics[i]); + numBytes += Long.BYTES; + } else if (aggregatedValueType.equals(DOUBLE)) { + if (isAggregatedDoc) { + long val = NumericUtils.doubleToSortableLong( + starTreeDocument.metrics[i] == null ? 0.0 : (Double) starTreeDocument.metrics[i] + ); + output.writeLong(val); + } else { output.writeLong(starTreeDocument.metrics[i] == null ? 0L : (Long) starTreeDocument.metrics[i]); - numBytes += Long.BYTES; - break; - case DOUBLE: - if (isAggregatedDoc) { - long val = NumericUtils.doubleToSortableLong( - starTreeDocument.metrics[i] == null ? 0.0 : (Double) starTreeDocument.metrics[i] - ); - output.writeLong(val); - } else { - output.writeLong(starTreeDocument.metrics[i] == null ? 0L : (Long) starTreeDocument.metrics[i]); - } - numBytes += Long.BYTES; - break; - default: - throw new IllegalStateException("Unsupported metric type"); + } + numBytes += Long.BYTES; + } else { + throw new IllegalStateException("Unsupported metric type"); } } numBytes += StarTreeDocumentBitSetUtil.writeBitSet(starTreeDocument.metrics, output); @@ -157,22 +158,20 @@ protected long readDimensions(Long[] dimensions, RandomAccessInput input, long o protected long readMetrics(RandomAccessInput input, long offset, int numMetrics, Object[] metrics, boolean isAggregatedDoc) throws IOException { for (int i = 0; i < numMetrics; i++) { - switch (metricAggregatorInfos.get(i).getValueAggregators().getAggregatedValueType()) { - case LONG: - metrics[i] = input.readLong(offset); - offset += Long.BYTES; - break; - case DOUBLE: - long val = input.readLong(offset); - if (isAggregatedDoc) { - metrics[i] = StarTreeNumericTypeConverters.sortableLongtoDouble(val); - } else { - metrics[i] = val; - } - offset += Long.BYTES; - break; - default: - throw new IllegalStateException("Unsupported metric type"); + FieldValueConverter aggregatedValueType = metricAggregatorInfos.get(i).getValueAggregators().getAggregatedValueType(); + if (aggregatedValueType.equals(LONG)) { + metrics[i] = input.readLong(offset); + offset += Long.BYTES; + } else if (aggregatedValueType.equals(DOUBLE)) { + long val = input.readLong(offset); + if (isAggregatedDoc) { + metrics[i] = DOUBLE.toDoubleValue(val); + } else { + metrics[i] = val; + } + offset += Long.BYTES; + } else { + throw new IllegalStateException("Unsupported metric type"); } } offset += StarTreeDocumentBitSetUtil.readBitSet(input, offset, metrics, index -> null); diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/builder/BaseStarTreeBuilder.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/builder/BaseStarTreeBuilder.java index 13d5059a241b1..ba4cb792c00df 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/builder/BaseStarTreeBuilder.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/builder/BaseStarTreeBuilder.java @@ -34,11 +34,11 @@ import org.opensearch.index.compositeindex.datacube.startree.node.InMemoryTreeNode; import org.opensearch.index.compositeindex.datacube.startree.node.StarTreeNodeType; import org.opensearch.index.compositeindex.datacube.startree.utils.SequentialDocValuesIterator; -import org.opensearch.index.fielddata.IndexNumericFieldData; import org.opensearch.index.mapper.DocCountFieldMapper; +import org.opensearch.index.mapper.FieldMapper; +import org.opensearch.index.mapper.FieldValueConverter; import org.opensearch.index.mapper.Mapper; import org.opensearch.index.mapper.MapperService; -import org.opensearch.index.mapper.NumberFieldMapper; import java.io.IOException; import java.util.ArrayList; @@ -55,6 +55,8 @@ import static org.opensearch.index.compositeindex.datacube.startree.utils.StarTreeUtils.fullyQualifiedFieldNameForStarTreeDimensionsDocValues; import static org.opensearch.index.compositeindex.datacube.startree.utils.StarTreeUtils.fullyQualifiedFieldNameForStarTreeMetricsDocValues; import static org.opensearch.index.compositeindex.datacube.startree.utils.StarTreeUtils.getFieldInfo; +import static org.opensearch.index.mapper.NumberFieldMapper.NumberType.DOUBLE; +import static org.opensearch.index.mapper.NumberFieldMapper.NumberType.LONG; /** * Builder for star tree. Defines the algorithm to construct star-tree @@ -143,7 +145,7 @@ public List generateMetricAggregatorInfos(MapperService ma MetricStat.DOC_COUNT, metric.getField(), starTreeField.getName(), - IndexNumericFieldData.NumericType.LONG + LONG ); metricAggregatorInfos.add(metricAggregatorInfo); continue; @@ -152,10 +154,10 @@ public List generateMetricAggregatorInfos(MapperService ma if (metricStat.isDerivedMetric()) { continue; } - IndexNumericFieldData.NumericType numericType; + FieldValueConverter fieldValueConverter; Mapper fieldMapper = mapperService.documentMapper().mappers().getMapper(metric.getField()); - if (fieldMapper instanceof NumberFieldMapper) { - numericType = ((NumberFieldMapper) fieldMapper).fieldType().numericType(); + if (fieldMapper instanceof FieldMapper && ((FieldMapper) fieldMapper).fieldType() instanceof FieldValueConverter) { + fieldValueConverter = (FieldValueConverter) ((FieldMapper) fieldMapper).fieldType(); } else { logger.error("unsupported mapper type"); throw new IllegalStateException("unsupported mapper type"); @@ -165,7 +167,7 @@ public List generateMetricAggregatorInfos(MapperService ma metricStat, metric.getField(), starTreeField.getName(), - numericType + fieldValueConverter ); metricAggregatorInfos.add(metricAggregatorInfo); } @@ -351,20 +353,17 @@ private void createSortedDocValuesIndices(DocValuesConsumer docValuesConsumer, A for (int i = 0; i < starTreeDocument.metrics.length; i++) { try { - switch (metricAggregatorInfos.get(i).getValueAggregators().getAggregatedValueType()) { - case LONG: - if (starTreeDocument.metrics[i] != null) { - metricWriters.get(i).addValue(docId, (long) starTreeDocument.metrics[i]); - } - break; - case DOUBLE: - if (starTreeDocument.metrics[i] != null) { - metricWriters.get(i) - .addValue(docId, NumericUtils.doubleToSortableLong((Double) starTreeDocument.metrics[i])); - } - break; - default: - throw new IllegalStateException("Unknown metric doc value type"); + FieldValueConverter aggregatedValueType = metricAggregatorInfos.get(i).getValueAggregators().getAggregatedValueType(); + if (aggregatedValueType.equals(LONG)) { + if (starTreeDocument.metrics[i] != null) { + metricWriters.get(i).addValue(docId, (long) starTreeDocument.metrics[i]); + } + } else if (aggregatedValueType.equals(DOUBLE)) { + if (starTreeDocument.metrics[i] != null) { + metricWriters.get(i).addValue(docId, NumericUtils.doubleToSortableLong((Double) starTreeDocument.metrics[i])); + } + } else { + throw new IllegalStateException("Unknown metric doc value type"); } } catch (IllegalArgumentException e) { logger.error("could not parse the value, exiting creation of star tree"); diff --git a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/fileformats/node/FixedLengthStarTreeNode.java b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/fileformats/node/FixedLengthStarTreeNode.java index 0ae704afa53b9..df2ce9096bfc1 100644 --- a/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/fileformats/node/FixedLengthStarTreeNode.java +++ b/server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/fileformats/node/FixedLengthStarTreeNode.java @@ -250,7 +250,7 @@ private FixedLengthStarTreeNode binarySearchChild(long dimensionValue) throws IO } int high = getInt(LAST_CHILD_ID_OFFSET); - // if the current node is null node, increment the low to reduce the search space + // if the current node is null node, decrement the high to reduce the search space if (matchStarTreeNodeTypeOrNull(new FixedLengthStarTreeNode(in, high), StarTreeNodeType.NULL) != null) { high--; } diff --git a/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java index e60d5a739a9ae..bc50f507010c4 100644 --- a/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java @@ -349,6 +349,7 @@ public DateFieldMapper build(BuilderContext context) { Long nullTimestamp = parseNullValue(ft); return new DateFieldMapper(name, ft, multiFieldsBuilder.build(this, context), copyTo.build(), nullTimestamp, resolution, this); } + } public static final TypeParser MILLIS_PARSER = new TypeParser((n, c) -> { diff --git a/server/src/main/java/org/opensearch/index/mapper/DocCountFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/DocCountFieldMapper.java index 0a461fff38fac..db4770adf6666 100644 --- a/server/src/main/java/org/opensearch/index/mapper/DocCountFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/DocCountFieldMapper.java @@ -75,6 +75,11 @@ protected List> getParameters() { public DocCountFieldMapper build(BuilderContext context) { return new DocCountFieldMapper(); } + + @Override + public boolean isDataCubeMetricSupported() { + return true; + } } /** diff --git a/server/src/main/java/org/opensearch/index/mapper/FieldValueConverter.java b/server/src/main/java/org/opensearch/index/mapper/FieldValueConverter.java new file mode 100644 index 0000000000000..cb16dba6ab640 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/mapper/FieldValueConverter.java @@ -0,0 +1,30 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.index.mapper; + +/** + *

+ * Implementations of this interface should define the conversion logic + * from a sortable long value to a double value, taking into account any necessary + * scaling, normalization, or other transformations required by the specific + * field type. + * + * @opensearch.experimental + */ +public interface FieldValueConverter { + + /** + * Converts the Lucene representation of the value as a long to an actual double representation + * + * @param value the long value to be converted + * @return the corresponding double value + */ + double toDoubleValue(long value); + +} diff --git a/server/src/main/java/org/opensearch/index/mapper/Mapper.java b/server/src/main/java/org/opensearch/index/mapper/Mapper.java index 46a5050d4fc18..87fdd8266a795 100644 --- a/server/src/main/java/org/opensearch/index/mapper/Mapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/Mapper.java @@ -100,7 +100,7 @@ public Version indexCreatedVersionOrDefault(@Nullable Version defaultValue) { * @opensearch.api */ @PublicApi(since = "1.0.0") - public abstract static class Builder { + public abstract static class Builder implements MapperBuilderProperties { public String name; diff --git a/server/src/main/java/org/opensearch/index/mapper/MapperBuilderProperties.java b/server/src/main/java/org/opensearch/index/mapper/MapperBuilderProperties.java new file mode 100644 index 0000000000000..ce7b8f28b0b29 --- /dev/null +++ b/server/src/main/java/org/opensearch/index/mapper/MapperBuilderProperties.java @@ -0,0 +1,41 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.index.mapper; + +import org.opensearch.index.compositeindex.datacube.DimensionType; + +import java.util.Optional; + +/** + * An interface that defines properties for MapperBuilder implementations. + * + * @opensearch.experimental + */ +public interface MapperBuilderProperties { + + /** + * Indicates whether the implementation supports data cube dimensions. + * + * @return an Optional containing the supported DimensionType if data cube dimensions are supported, + * or an empty Optional if not supported + */ + default Optional getSupportedDataCubeDimensionType() { + return Optional.empty(); + } + + /** + * Indicates whether the implementation supports data cube metrics. + * + * @return true if data cube metrics are supported, false otherwise + */ + default boolean isDataCubeMetricSupported() { + return false; + } + +} diff --git a/server/src/main/java/org/opensearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/NumberFieldMapper.java index 9286b5c64b5f2..eb0694edc70ba 100644 --- a/server/src/main/java/org/opensearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/NumberFieldMapper.java @@ -63,6 +63,7 @@ import org.opensearch.core.common.bytes.BytesArray; import org.opensearch.core.xcontent.XContentParser; import org.opensearch.core.xcontent.XContentParser.Token; +import org.opensearch.index.compositeindex.datacube.DimensionType; import org.opensearch.index.document.SortedUnsignedLongDocValuesRangeQuery; import org.opensearch.index.document.SortedUnsignedLongDocValuesSetQuery; import org.opensearch.index.fielddata.IndexFieldData; @@ -84,6 +85,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.function.BiFunction; import java.util.function.Function; import java.util.function.Supplier; @@ -172,6 +174,22 @@ public NumberFieldMapper build(BuilderContext context) { MappedFieldType ft = new NumberFieldType(buildFullName(context), this); return new NumberFieldMapper(name, ft, multiFieldsBuilder.build(this, context), copyTo.build(), this); } + + @Override + public Optional getSupportedDataCubeDimensionType() { + + // unsigned long is not supported as dimension for star tree + if (type.numericType.equals(NumericType.UNSIGNED_LONG)) { + return Optional.empty(); + } + + return Optional.of(DimensionType.NUMERIC); + } + + @Override + public boolean isDataCubeMetricSupported() { + return true; + } } /** @@ -179,7 +197,7 @@ public NumberFieldMapper build(BuilderContext context) { * * @opensearch.internal */ - public enum NumberType implements NumericPointEncoder { + public enum NumberType implements NumericPointEncoder, FieldValueConverter { HALF_FLOAT("half_float", NumericType.HALF_FLOAT) { @Override public Float parse(Object value, boolean coerce) { @@ -209,6 +227,11 @@ public byte[] encodePoint(Number value) { return point; } + @Override + public double toDoubleValue(long value) { + return HalfFloatPoint.sortableShortToHalfFloat((short) value); + } + @Override public Float parse(XContentParser parser, boolean coerce) throws IOException { float parsed = parser.floatValue(coerce); @@ -353,6 +376,11 @@ public byte[] encodePoint(Number value) { return point; } + @Override + public double toDoubleValue(long value) { + return NumericUtils.sortableIntToFloat((int) value); + } + @Override public Float parse(XContentParser parser, boolean coerce) throws IOException { float parsed = parser.floatValue(coerce); @@ -486,6 +514,11 @@ public byte[] encodePoint(Number value) { return point; } + @Override + public double toDoubleValue(long value) { + return NumericUtils.sortableLongToDouble(value); + } + @Override public Double parse(XContentParser parser, boolean coerce) throws IOException { double parsed = parser.doubleValue(coerce); @@ -618,6 +651,13 @@ public byte[] encodePoint(Number value) { return point; } + @Override + public double toDoubleValue(long value) { + byte[] bytes = new byte[8]; + NumericUtils.longToSortableBytes(value, bytes, 0); + return NumericUtils.sortableLongToDouble(NumericUtils.sortableBytesToLong(bytes, 0)); + } + @Override public Short parse(XContentParser parser, boolean coerce) throws IOException { int value = parser.intValue(coerce); @@ -697,6 +737,11 @@ public byte[] encodePoint(Number value) { return point; } + @Override + public double toDoubleValue(long value) { + return (double) value; + } + @Override public Short parse(XContentParser parser, boolean coerce) throws IOException { return parser.shortValue(coerce); @@ -772,6 +817,11 @@ public byte[] encodePoint(Number value) { return point; } + @Override + public double toDoubleValue(long value) { + return (double) value; + } + @Override public Integer parse(XContentParser parser, boolean coerce) throws IOException { return parser.intValue(coerce); @@ -943,6 +993,11 @@ public byte[] encodePoint(Number value) { return point; } + @Override + public double toDoubleValue(long value) { + return (double) value; + } + @Override public Long parse(XContentParser parser, boolean coerce) throws IOException { return parser.longValue(coerce); @@ -1070,6 +1125,11 @@ public byte[] encodePoint(Number value) { return point; } + @Override + public double toDoubleValue(long value) { + return Numbers.unsignedLongToDouble(value); + } + @Override public BigInteger parse(XContentParser parser, boolean coerce) throws IOException { return parser.bigIntegerValue(coerce); @@ -1182,12 +1242,16 @@ Number valueForSearch(String value) { this.parser = new TypeParser((n, c) -> new Builder(n, this, c.getSettings())); } - /** Get the associated type name. */ + /** + * Get the associated type name. + */ public final String typeName() { return name; } - /** Get the associated numeric type */ + /** + * Get the associated numeric type + */ public final NumericType numericType() { return numericType; } @@ -1486,7 +1550,7 @@ protected String toString(byte[] value) { * * @opensearch.internal */ - public static class NumberFieldType extends SimpleMappedFieldType implements NumericPointEncoder { + public static class NumberFieldType extends SimpleMappedFieldType implements NumericPointEncoder, FieldValueConverter { private final NumberType type; private final boolean coerce; @@ -1652,6 +1716,11 @@ public Number parsePoint(byte[] value) { public byte[] encodePoint(Number value) { return type.encodePoint(value); } + + @Override + public double toDoubleValue(long value) { + return type.toDoubleValue(value); + } } private final NumberType type; diff --git a/server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java b/server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java index 17c27ef149e54..9e2e7ef3c3889 100644 --- a/server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/StarTreeMapper.java @@ -62,11 +62,6 @@ public ParametrizedFieldMapper.Builder getMergeBuilder() { */ public static class Builder extends ParametrizedFieldMapper.Builder { private ObjectMapper.Builder objbuilder; - private static final Set> ALLOWED_DIMENSION_MAPPER_BUILDERS = Set.of( - NumberFieldMapper.Builder.class, - DateFieldMapper.Builder.class - ); - private static final Set> ALLOWED_METRIC_MAPPER_BUILDERS = Set.of(NumberFieldMapper.Builder.class); @SuppressWarnings("unchecked") private final Parameter config = new Parameter<>(CONFIG, false, () -> null, (name, context, nodeObj) -> { @@ -380,11 +375,11 @@ protected List> getParameters() { } private static boolean isBuilderAllowedForDimension(Mapper.Builder builder) { - return ALLOWED_DIMENSION_MAPPER_BUILDERS.stream().anyMatch(allowedType -> allowedType.isInstance(builder)); + return builder.getSupportedDataCubeDimensionType().isPresent(); } private static boolean isBuilderAllowedForMetric(Mapper.Builder builder) { - return ALLOWED_METRIC_MAPPER_BUILDERS.stream().anyMatch(allowedType -> allowedType.isInstance(builder)); + return builder.isDataCubeMetricSupported(); } private Optional findMapperBuilderByName(String field, List mappersBuilders) { diff --git a/server/src/test/java/org/opensearch/index/codec/composite99/datacube/startree/StarTreeDocValuesFormatTests.java b/server/src/test/java/org/opensearch/index/codec/composite99/datacube/startree/StarTreeDocValuesFormatTests.java index 4bbefeba0845b..0c6d21d28cc8a 100644 --- a/server/src/test/java/org/opensearch/index/codec/composite99/datacube/startree/StarTreeDocValuesFormatTests.java +++ b/server/src/test/java/org/opensearch/index/codec/composite99/datacube/startree/StarTreeDocValuesFormatTests.java @@ -46,9 +46,9 @@ import org.opensearch.index.compositeindex.datacube.startree.StarTreeFieldConfiguration; import org.opensearch.index.compositeindex.datacube.startree.StarTreeIndexSettings; import org.opensearch.index.compositeindex.datacube.startree.StarTreeTestUtils; -import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType; import org.opensearch.index.compositeindex.datacube.startree.index.StarTreeValues; import org.opensearch.index.mapper.MapperService; +import org.opensearch.index.mapper.NumberFieldMapper; import org.opensearch.indices.IndicesModule; import org.junit.After; import org.junit.AfterClass; @@ -159,7 +159,7 @@ public void testStarTreeDocValues() throws IOException { StarTreeValues starTreeValues = (StarTreeValues) starTreeDocValuesReader.getCompositeIndexValues(compositeIndexFieldInfo); StarTreeDocument[] starTreeDocuments = StarTreeTestUtils.getSegmentsStarTreeDocuments( List.of(starTreeValues), - List.of(StarTreeNumericType.DOUBLE, StarTreeNumericType.LONG, StarTreeNumericType.LONG), + List.of(NumberFieldMapper.NumberType.DOUBLE, NumberFieldMapper.NumberType.LONG, NumberFieldMapper.NumberType.LONG), reader.maxDoc() ); assertStarTreeDocuments(starTreeDocuments, expectedStarTreeDocuments); diff --git a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeTestUtils.java b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeTestUtils.java index 7b5672529dc8d..f3e111bf6caa6 100644 --- a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeTestUtils.java +++ b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeTestUtils.java @@ -12,7 +12,6 @@ import org.opensearch.index.compositeindex.datacube.Dimension; import org.opensearch.index.compositeindex.datacube.Metric; import org.opensearch.index.compositeindex.datacube.MetricStat; -import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType; import org.opensearch.index.compositeindex.datacube.startree.fileformats.meta.StarTreeMetadata; import org.opensearch.index.compositeindex.datacube.startree.index.StarTreeValues; import org.opensearch.index.compositeindex.datacube.startree.node.InMemoryTreeNode; @@ -21,6 +20,7 @@ import org.opensearch.index.compositeindex.datacube.startree.node.StarTreeNodeType; import org.opensearch.index.compositeindex.datacube.startree.utils.SequentialDocValuesIterator; import org.opensearch.index.mapper.CompositeMappedFieldType; +import org.opensearch.index.mapper.FieldValueConverter; import java.io.IOException; import java.util.ArrayDeque; @@ -44,7 +44,7 @@ public class StarTreeTestUtils { public static StarTreeDocument[] getSegmentsStarTreeDocuments( List starTreeValuesSubs, - List starTreeNumericTypes, + List fieldValueConverters, int numDocs ) throws IOException { List starTreeDocuments = new ArrayList<>(); @@ -72,7 +72,7 @@ public static StarTreeDocument[] getSegmentsStarTreeDocuments( } int currentDocId = 0; while (currentDocId < numDocs) { - starTreeDocuments.add(getStarTreeDocument(currentDocId, dimensionReaders, metricReaders, starTreeNumericTypes)); + starTreeDocuments.add(getStarTreeDocument(currentDocId, dimensionReaders, metricReaders, fieldValueConverters)); currentDocId++; } } @@ -84,7 +84,7 @@ public static StarTreeDocument getStarTreeDocument( int currentDocId, SequentialDocValuesIterator[] dimensionReaders, List metricReaders, - List starTreeNumericTypes + List fieldValueConverters ) throws IOException { Long[] dims = new Long[dimensionReaders.length]; int i = 0; @@ -98,15 +98,15 @@ public static StarTreeDocument getStarTreeDocument( Object[] metrics = new Object[metricReaders.size()]; for (SequentialDocValuesIterator metricDocValuesIterator : metricReaders) { metricDocValuesIterator.nextDoc(currentDocId); - metrics[i] = toStarTreeNumericTypeValue(metricDocValuesIterator.value(currentDocId), starTreeNumericTypes.get(i)); + metrics[i] = toAggregatorValueType(metricDocValuesIterator.value(currentDocId), fieldValueConverters.get(i)); i++; } return new StarTreeDocument(dims, metrics); } - public static Double toStarTreeNumericTypeValue(Long value, StarTreeNumericType starTreeNumericType) { + public static Double toAggregatorValueType(Long value, FieldValueConverter fieldValueConverter) { try { - return starTreeNumericType.getDoubleValue(value); + return fieldValueConverter.toDoubleValue(value); } catch (Exception e) { throw new IllegalStateException("Cannot convert " + value + " to sortable aggregation type", e); } diff --git a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/AbstractValueAggregatorTests.java b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/AbstractValueAggregatorTests.java index 36f75834abba8..00f9fcf0e269f 100644 --- a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/AbstractValueAggregatorTests.java +++ b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/AbstractValueAggregatorTests.java @@ -10,7 +10,8 @@ import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; -import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType; +import org.opensearch.index.mapper.FieldValueConverter; +import org.opensearch.index.mapper.NumberFieldMapper; import org.opensearch.test.OpenSearchTestCase; import org.junit.Before; @@ -21,27 +22,27 @@ public abstract class AbstractValueAggregatorTests extends OpenSearchTestCase { private ValueAggregator aggregator; - protected StarTreeNumericType starTreeNumericType; + protected FieldValueConverter fieldValueConverter; - public AbstractValueAggregatorTests(StarTreeNumericType starTreeNumericType) { - this.starTreeNumericType = starTreeNumericType; + public AbstractValueAggregatorTests(FieldValueConverter fieldValueConverter) { + this.fieldValueConverter = fieldValueConverter; } @Before public void setup() { - aggregator = getValueAggregator(starTreeNumericType); + aggregator = getValueAggregator(fieldValueConverter); } @ParametersFactory public static Collection parameters() { List parameters = new ArrayList<>(); - for (StarTreeNumericType starTreeNumericType : StarTreeNumericType.values()) { - parameters.add(new Object[] { starTreeNumericType }); + for (FieldValueConverter fieldValueConverter : NumberFieldMapper.NumberType.values()) { + parameters.add(new Object[] { fieldValueConverter }); } return parameters; } - public abstract ValueAggregator getValueAggregator(StarTreeNumericType starTreeNumericType); + public abstract ValueAggregator getValueAggregator(FieldValueConverter fieldValueConverter); public void testGetInitialAggregatedValueForSegmentDocNullValue() { assertEquals(aggregator.getIdentityMetricValue(), aggregator.getInitialAggregatedValueForSegmentDocValue(null)); @@ -61,6 +62,10 @@ public void testGetInitialAggregatedNullValue() { public void testGetInitialAggregatedValueForSegmentDocValue() { long randomLong = randomLong(); - assertEquals(starTreeNumericType.getDoubleValue(randomLong), aggregator.getInitialAggregatedValueForSegmentDocValue(randomLong)); + if (aggregator instanceof CountValueAggregator) { + assertEquals(CountValueAggregator.DEFAULT_INITIAL_VALUE, aggregator.getInitialAggregatedValueForSegmentDocValue(randomLong())); + } else { + assertEquals(fieldValueConverter.toDoubleValue(randomLong), aggregator.getInitialAggregatedValueForSegmentDocValue(randomLong)); + } } } diff --git a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/CountValueAggregatorTests.java b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/CountValueAggregatorTests.java index b270c1b1bc26c..db22ee9af18e2 100644 --- a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/CountValueAggregatorTests.java +++ b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/CountValueAggregatorTests.java @@ -8,14 +8,14 @@ package org.opensearch.index.compositeindex.datacube.startree.aggregators; -import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType; +import org.opensearch.index.mapper.FieldValueConverter; public class CountValueAggregatorTests extends AbstractValueAggregatorTests { private CountValueAggregator aggregator; - public CountValueAggregatorTests(StarTreeNumericType starTreeNumericType) { - super(starTreeNumericType); + public CountValueAggregatorTests(FieldValueConverter fieldValueConverter) { + super(fieldValueConverter); } public void testMergeAggregatedValueAndSegmentValue() { @@ -52,7 +52,7 @@ public void testIdentityMetricValue() { } @Override - public ValueAggregator getValueAggregator(StarTreeNumericType starTreeNumericType) { + public ValueAggregator getValueAggregator(FieldValueConverter fieldValueConverter) { aggregator = new CountValueAggregator(); return aggregator; } diff --git a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/DocCountAggregatorTests.java b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/DocCountAggregatorTests.java index 2765629aa5950..3960412d65868 100644 --- a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/DocCountAggregatorTests.java +++ b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/DocCountAggregatorTests.java @@ -8,7 +8,7 @@ package org.opensearch.index.compositeindex.datacube.startree.aggregators; -import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType; +import org.opensearch.index.mapper.FieldValueConverter; /** * Unit tests for {@link DocCountAggregator}. @@ -17,8 +17,8 @@ public class DocCountAggregatorTests extends AbstractValueAggregatorTests { private DocCountAggregator aggregator; - public DocCountAggregatorTests(StarTreeNumericType starTreeNumericType) { - super(starTreeNumericType); + public DocCountAggregatorTests(FieldValueConverter fieldValueConverter) { + super(fieldValueConverter); } public void testMergeAggregatedValueAndSegmentValue() { @@ -52,7 +52,7 @@ public void testGetInitialAggregatedValue() { assertEquals(randomLong, (long) aggregator.getInitialAggregatedValue(randomLong)); } - public void testToStarTreeNumericTypeValue() { + public void testToAggregatedValueType() { long randomLong = randomLong(); assertEquals(randomLong, (long) aggregator.toAggregatedValueType(randomLong)); } @@ -62,7 +62,7 @@ public void testIdentityMetricValue() { } @Override - public ValueAggregator getValueAggregator(StarTreeNumericType starTreeNumericType) { + public ValueAggregator getValueAggregator(FieldValueConverter fieldValueConverter) { aggregator = new DocCountAggregator(); return aggregator; } diff --git a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MaxValueAggregatorTests.java b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MaxValueAggregatorTests.java index b103416251c46..b713a47ed5f62 100644 --- a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MaxValueAggregatorTests.java +++ b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MaxValueAggregatorTests.java @@ -9,27 +9,28 @@ package org.opensearch.index.compositeindex.datacube.startree.aggregators; import org.apache.lucene.util.NumericUtils; -import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType; +import org.opensearch.index.mapper.FieldValueConverter; +import org.opensearch.index.mapper.NumberFieldMapper; public class MaxValueAggregatorTests extends AbstractValueAggregatorTests { private MaxValueAggregator aggregator; - public MaxValueAggregatorTests(StarTreeNumericType starTreeNumericType) { - super(starTreeNumericType); + public MaxValueAggregatorTests(FieldValueConverter fieldValueConverter) { + super(fieldValueConverter); } public void testMergeAggregatedValueAndSegmentValue() { Long randomLong = randomLong(); double randomDouble = randomDouble(); assertEquals( - Math.max(starTreeNumericType.getDoubleValue(randomLong), randomDouble), + Math.max(fieldValueConverter.toDoubleValue(randomLong), randomDouble), aggregator.mergeAggregatedValueAndSegmentValue(randomDouble, randomLong), 0.0 ); - assertEquals(starTreeNumericType.getDoubleValue(randomLong), aggregator.mergeAggregatedValueAndSegmentValue(null, randomLong), 0.0); + assertEquals(fieldValueConverter.toDoubleValue(randomLong), aggregator.mergeAggregatedValueAndSegmentValue(null, randomLong), 0.0); assertEquals(randomDouble, aggregator.mergeAggregatedValueAndSegmentValue(randomDouble, null), 0.0); - assertEquals(Math.max(2.0, starTreeNumericType.getDoubleValue(3L)), aggregator.mergeAggregatedValueAndSegmentValue(2.0, 3L), 0.0); + assertEquals(Math.max(2.0, fieldValueConverter.toDoubleValue(3L)), aggregator.mergeAggregatedValueAndSegmentValue(2.0, 3L), 0.0); } public void testMergeAggregatedValues() { @@ -46,7 +47,7 @@ public void testGetInitialAggregatedValue() { } public void testToAggregatedValueType() { - MaxValueAggregator aggregator = new MaxValueAggregator(StarTreeNumericType.DOUBLE); + MaxValueAggregator aggregator = new MaxValueAggregator(NumberFieldMapper.NumberType.DOUBLE); long randomLong = randomLong(); assertEquals(NumericUtils.sortableLongToDouble(randomLong), aggregator.toAggregatedValueType(randomLong), 0.0); } @@ -56,8 +57,8 @@ public void testIdentityMetricValue() { } @Override - public ValueAggregator getValueAggregator(StarTreeNumericType starTreeNumericType) { - aggregator = new MaxValueAggregator(starTreeNumericType); + public ValueAggregator getValueAggregator(FieldValueConverter fieldValueConverter) { + aggregator = new MaxValueAggregator(fieldValueConverter); return aggregator; } diff --git a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MetricAggregatorInfoTests.java b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MetricAggregatorInfoTests.java index 62671ffa03b82..5588cc1b366c9 100644 --- a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MetricAggregatorInfoTests.java +++ b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MetricAggregatorInfoTests.java @@ -9,7 +9,7 @@ package org.opensearch.index.compositeindex.datacube.startree.aggregators; import org.opensearch.index.compositeindex.datacube.MetricStat; -import org.opensearch.index.fielddata.IndexNumericFieldData; +import org.opensearch.index.mapper.NumberFieldMapper; import org.opensearch.test.OpenSearchTestCase; public class MetricAggregatorInfoTests extends OpenSearchTestCase { @@ -19,7 +19,7 @@ public void testConstructor() { MetricStat.SUM, "column1", "star_tree_field", - IndexNumericFieldData.NumericType.DOUBLE + NumberFieldMapper.NumberType.DOUBLE ); assertEquals(MetricStat.SUM, pair.getMetricStat()); assertEquals("column1", pair.getField()); @@ -30,7 +30,7 @@ public void testCountStarConstructor() { MetricStat.VALUE_COUNT, "anything", "star_tree_field", - IndexNumericFieldData.NumericType.DOUBLE + NumberFieldMapper.NumberType.DOUBLE ); assertEquals(MetricStat.VALUE_COUNT, pair.getMetricStat()); assertEquals("anything", pair.getField()); @@ -41,7 +41,7 @@ public void testToFieldName() { MetricStat.SUM, "column2", "star_tree_field", - IndexNumericFieldData.NumericType.DOUBLE + NumberFieldMapper.NumberType.DOUBLE ); assertEquals("star_tree_field_column2_sum", pair.toFieldName()); } @@ -51,23 +51,20 @@ public void testEquals() { MetricStat.SUM, "column1", "star_tree_field", - IndexNumericFieldData.NumericType.DOUBLE + NumberFieldMapper.NumberType.DOUBLE ); MetricAggregatorInfo pair2 = new MetricAggregatorInfo( MetricStat.SUM, "column1", "star_tree_field", - IndexNumericFieldData.NumericType.DOUBLE + NumberFieldMapper.NumberType.DOUBLE ); assertEquals(pair1, pair2); assertNotEquals( pair1, - new MetricAggregatorInfo(MetricStat.VALUE_COUNT, "column1", "star_tree_field", IndexNumericFieldData.NumericType.DOUBLE) - ); - assertNotEquals( - pair1, - new MetricAggregatorInfo(MetricStat.SUM, "column2", "star_tree_field", IndexNumericFieldData.NumericType.DOUBLE) + new MetricAggregatorInfo(MetricStat.VALUE_COUNT, "column1", "star_tree_field", NumberFieldMapper.NumberType.DOUBLE) ); + assertNotEquals(pair1, new MetricAggregatorInfo(MetricStat.SUM, "column2", "star_tree_field", NumberFieldMapper.NumberType.DOUBLE)); } public void testHashCode() { @@ -75,13 +72,13 @@ public void testHashCode() { MetricStat.SUM, "column1", "star_tree_field", - IndexNumericFieldData.NumericType.DOUBLE + NumberFieldMapper.NumberType.DOUBLE ); MetricAggregatorInfo pair2 = new MetricAggregatorInfo( MetricStat.SUM, "column1", "star_tree_field", - IndexNumericFieldData.NumericType.DOUBLE + NumberFieldMapper.NumberType.DOUBLE ); assertEquals(pair1.hashCode(), pair2.hashCode()); } @@ -91,19 +88,19 @@ public void testCompareTo() { MetricStat.SUM, "column1", "star_tree_field", - IndexNumericFieldData.NumericType.DOUBLE + NumberFieldMapper.NumberType.DOUBLE ); MetricAggregatorInfo pair2 = new MetricAggregatorInfo( MetricStat.SUM, "column2", "star_tree_field", - IndexNumericFieldData.NumericType.DOUBLE + NumberFieldMapper.NumberType.DOUBLE ); MetricAggregatorInfo pair3 = new MetricAggregatorInfo( MetricStat.VALUE_COUNT, "column1", "star_tree_field", - IndexNumericFieldData.NumericType.DOUBLE + NumberFieldMapper.NumberType.DOUBLE ); assertTrue(pair1.compareTo(pair2) < 0); assertTrue(pair2.compareTo(pair1) > 0); diff --git a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MinValueAggregatorTests.java b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MinValueAggregatorTests.java index 013c60d8a1b91..ec406d71ff633 100644 --- a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MinValueAggregatorTests.java +++ b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MinValueAggregatorTests.java @@ -9,26 +9,27 @@ package org.opensearch.index.compositeindex.datacube.startree.aggregators; import org.apache.lucene.util.NumericUtils; -import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType; +import org.opensearch.index.mapper.FieldValueConverter; +import org.opensearch.index.mapper.NumberFieldMapper; public class MinValueAggregatorTests extends AbstractValueAggregatorTests { private MinValueAggregator aggregator; - public MinValueAggregatorTests(StarTreeNumericType starTreeNumericType) { - super(starTreeNumericType); + public MinValueAggregatorTests(FieldValueConverter fieldValueConverter) { + super(fieldValueConverter); } public void testMergeAggregatedValueAndSegmentValue() { Long randomLong = randomLong(); double randomDouble = randomDouble(); assertEquals( - Math.min(starTreeNumericType.getDoubleValue(randomLong), randomDouble), + Math.min(fieldValueConverter.toDoubleValue(randomLong), randomDouble), aggregator.mergeAggregatedValueAndSegmentValue(randomDouble, randomLong), 0.0 ); - assertEquals(starTreeNumericType.getDoubleValue(randomLong), aggregator.mergeAggregatedValueAndSegmentValue(null, randomLong), 0.0); + assertEquals(fieldValueConverter.toDoubleValue(randomLong), aggregator.mergeAggregatedValueAndSegmentValue(null, randomLong), 0.0); assertEquals(randomDouble, aggregator.mergeAggregatedValueAndSegmentValue(randomDouble, null), 0.0); - assertEquals(Math.min(2.0, starTreeNumericType.getDoubleValue(3L)), aggregator.mergeAggregatedValueAndSegmentValue(2.0, 3L), 0.0); + assertEquals(Math.min(2.0, fieldValueConverter.toDoubleValue(3L)), aggregator.mergeAggregatedValueAndSegmentValue(2.0, 3L), 0.0); } public void testMergeAggregatedValues() { @@ -45,7 +46,7 @@ public void testGetInitialAggregatedValue() { } public void testToAggregatedValueType() { - MinValueAggregator aggregator = new MinValueAggregator(StarTreeNumericType.DOUBLE); + MinValueAggregator aggregator = new MinValueAggregator(NumberFieldMapper.NumberType.DOUBLE); long randomLong = randomLong(); assertEquals(NumericUtils.sortableLongToDouble(randomLong), aggregator.toAggregatedValueType(randomLong), 0.0); } @@ -55,8 +56,8 @@ public void testIdentityMetricValue() { } @Override - public ValueAggregator getValueAggregator(StarTreeNumericType starTreeNumericType) { - aggregator = new MinValueAggregator(starTreeNumericType); + public ValueAggregator getValueAggregator(FieldValueConverter fieldValueConverter) { + aggregator = new MinValueAggregator(fieldValueConverter); return aggregator; } diff --git a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/StaticValueAggregatorTests.java b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/StaticValueAggregatorTests.java index 487bd54602503..2ba29a1165dc8 100644 --- a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/StaticValueAggregatorTests.java +++ b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/StaticValueAggregatorTests.java @@ -9,7 +9,7 @@ package org.opensearch.index.compositeindex.datacube.startree.aggregators; import org.apache.lucene.util.NumericUtils; -import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType; +import org.opensearch.index.mapper.NumberFieldMapper; import org.opensearch.search.aggregations.metrics.CompensatedSum; import org.opensearch.test.OpenSearchTestCase; @@ -39,7 +39,7 @@ public void testKahanSummation() { private static double getAggregatedValue(double[] numbers) { // explicitly took double to test for most precision // hard to run similar tests for different data types dynamically as inputs and precision vary - SumValueAggregator aggregator = new SumValueAggregator(StarTreeNumericType.DOUBLE); + SumValueAggregator aggregator = new SumValueAggregator(NumberFieldMapper.NumberType.DOUBLE); double aggregatedValue = aggregator.getInitialAggregatedValueForSegmentDocValue(NumericUtils.doubleToSortableLong(numbers[0])); aggregatedValue = aggregator.mergeAggregatedValueAndSegmentValue(aggregatedValue, NumericUtils.doubleToSortableLong(numbers[1])); aggregatedValue = aggregator.mergeAggregatedValueAndSegmentValue(aggregatedValue, NumericUtils.doubleToSortableLong(numbers[2])); @@ -65,7 +65,7 @@ private double normalSum(double[] numbers) { public void testMaxAggregatorExtremeValues() { double[] numbers = { Double.MAX_VALUE, Double.MIN_VALUE, 0.0, Double.MAX_VALUE + 1 }; double expected = Double.MAX_VALUE + 1; - MaxValueAggregator aggregator = new MaxValueAggregator(StarTreeNumericType.DOUBLE); + MaxValueAggregator aggregator = new MaxValueAggregator(NumberFieldMapper.NumberType.DOUBLE); double aggregatedValue = aggregator.getInitialAggregatedValueForSegmentDocValue(NumericUtils.doubleToSortableLong(numbers[0])); for (int i = 1; i < numbers.length; i++) { aggregatedValue = aggregator.mergeAggregatedValueAndSegmentValue( @@ -85,7 +85,7 @@ public void testMaxAggregatorExtremeValues_Infinity() { Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY }; double expected = Double.POSITIVE_INFINITY; - MaxValueAggregator aggregator = new MaxValueAggregator(StarTreeNumericType.DOUBLE); + MaxValueAggregator aggregator = new MaxValueAggregator(NumberFieldMapper.NumberType.DOUBLE); double aggregatedValue = aggregator.getInitialAggregatedValueForSegmentDocValue(NumericUtils.doubleToSortableLong(numbers[0])); for (int i = 1; i < numbers.length; i++) { aggregatedValue = aggregator.mergeAggregatedValueAndSegmentValue( @@ -99,7 +99,7 @@ public void testMaxAggregatorExtremeValues_Infinity() { public void testMinAggregatorExtremeValues() { double[] numbers = { Double.MAX_VALUE, Double.MIN_VALUE - 1, 0.0, Double.MAX_VALUE + 1 }; double expected = Double.MIN_VALUE - 1; - MinValueAggregator aggregator = new MinValueAggregator(StarTreeNumericType.DOUBLE); + MinValueAggregator aggregator = new MinValueAggregator(NumberFieldMapper.NumberType.DOUBLE); double aggregatedValue = aggregator.getInitialAggregatedValueForSegmentDocValue(NumericUtils.doubleToSortableLong(numbers[0])); for (int i = 1; i < numbers.length; i++) { aggregatedValue = aggregator.mergeAggregatedValueAndSegmentValue( @@ -119,7 +119,7 @@ public void testMinAggregatorExtremeValues_Infinity() { Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY }; double expected = Double.NEGATIVE_INFINITY; - MinValueAggregator aggregator = new MinValueAggregator(StarTreeNumericType.DOUBLE); + MinValueAggregator aggregator = new MinValueAggregator(NumberFieldMapper.NumberType.DOUBLE); double aggregatedValue = aggregator.getInitialAggregatedValueForSegmentDocValue(NumericUtils.doubleToSortableLong(numbers[0])); for (int i = 1; i < numbers.length; i++) { aggregatedValue = aggregator.mergeAggregatedValueAndSegmentValue( diff --git a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/SumValueAggregatorTests.java b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/SumValueAggregatorTests.java index 44c7f17a276b4..ef328fe7eafc2 100644 --- a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/SumValueAggregatorTests.java +++ b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/SumValueAggregatorTests.java @@ -8,19 +8,19 @@ package org.opensearch.index.compositeindex.datacube.startree.aggregators; -import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType; +import org.opensearch.index.mapper.FieldValueConverter; public class SumValueAggregatorTests extends AbstractValueAggregatorTests { private SumValueAggregator aggregator; - public SumValueAggregatorTests(StarTreeNumericType starTreeNumericType) { - super(starTreeNumericType); + public SumValueAggregatorTests(FieldValueConverter fieldValueConverter) { + super(fieldValueConverter); } @Override - public ValueAggregator getValueAggregator(StarTreeNumericType starTreeNumericType) { - aggregator = new SumValueAggregator(starTreeNumericType); + public ValueAggregator getValueAggregator(FieldValueConverter fieldValueConverter) { + aggregator = new SumValueAggregator(fieldValueConverter); return aggregator; } @@ -29,7 +29,7 @@ public void testMergeAggregatedValueAndSegmentValue() { Long randomLong = randomLong(); aggregator.getInitialAggregatedValue(randomDouble); assertEquals( - randomDouble + starTreeNumericType.getDoubleValue(randomLong), + randomDouble + fieldValueConverter.toDoubleValue(randomLong), aggregator.mergeAggregatedValueAndSegmentValue(randomDouble, randomLong), 0.0 ); @@ -41,7 +41,7 @@ public void testMergeAggregatedValueAndSegmentValue_nullSegmentDocValue() { aggregator.getInitialAggregatedValue(randomDouble1); assertEquals(randomDouble1, aggregator.mergeAggregatedValueAndSegmentValue(randomDouble1, null), 0.0); assertEquals( - randomDouble1 + starTreeNumericType.getDoubleValue(randomLong), + randomDouble1 + fieldValueConverter.toDoubleValue(randomLong), aggregator.mergeAggregatedValueAndSegmentValue(randomDouble1, randomLong), 0.0 ); @@ -50,7 +50,7 @@ public void testMergeAggregatedValueAndSegmentValue_nullSegmentDocValue() { public void testMergeAggregatedValueAndSegmentValue_nullInitialDocValue() { Long randomLong = randomLong(); aggregator.getInitialAggregatedValue(null); - assertEquals(starTreeNumericType.getDoubleValue(randomLong), aggregator.mergeAggregatedValueAndSegmentValue(null, randomLong), 0.0); + assertEquals(fieldValueConverter.toDoubleValue(randomLong), aggregator.mergeAggregatedValueAndSegmentValue(null, randomLong), 0.0); } public void testMergeAggregatedValues() { diff --git a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/ValueAggregatorFactoryTests.java b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/ValueAggregatorFactoryTests.java index 6572d75d5b738..96ab1b3a52e8b 100644 --- a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/ValueAggregatorFactoryTests.java +++ b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/ValueAggregatorFactoryTests.java @@ -9,31 +9,31 @@ package org.opensearch.index.compositeindex.datacube.startree.aggregators; import org.opensearch.index.compositeindex.datacube.MetricStat; -import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType; +import org.opensearch.index.mapper.NumberFieldMapper; import org.opensearch.test.OpenSearchTestCase; public class ValueAggregatorFactoryTests extends OpenSearchTestCase { public void testGetValueAggregatorForSumType() { - ValueAggregator aggregator = ValueAggregatorFactory.getValueAggregator(MetricStat.SUM, StarTreeNumericType.LONG); + ValueAggregator aggregator = ValueAggregatorFactory.getValueAggregator(MetricStat.SUM, NumberFieldMapper.NumberType.LONG); assertNotNull(aggregator); assertEquals(SumValueAggregator.class, aggregator.getClass()); } public void testGetValueAggregatorForMinType() { - ValueAggregator aggregator = ValueAggregatorFactory.getValueAggregator(MetricStat.MIN, StarTreeNumericType.LONG); + ValueAggregator aggregator = ValueAggregatorFactory.getValueAggregator(MetricStat.MIN, NumberFieldMapper.NumberType.LONG); assertNotNull(aggregator); assertEquals(MinValueAggregator.class, aggregator.getClass()); } public void testGetValueAggregatorForMaxType() { - ValueAggregator aggregator = ValueAggregatorFactory.getValueAggregator(MetricStat.MAX, StarTreeNumericType.LONG); + ValueAggregator aggregator = ValueAggregatorFactory.getValueAggregator(MetricStat.MAX, NumberFieldMapper.NumberType.LONG); assertNotNull(aggregator); assertEquals(MaxValueAggregator.class, aggregator.getClass()); } public void testGetValueAggregatorForCountType() { - ValueAggregator aggregator = ValueAggregatorFactory.getValueAggregator(MetricStat.VALUE_COUNT, StarTreeNumericType.LONG); + ValueAggregator aggregator = ValueAggregatorFactory.getValueAggregator(MetricStat.VALUE_COUNT, NumberFieldMapper.NumberType.LONG); assertNotNull(aggregator); assertEquals(CountValueAggregator.class, aggregator.getClass()); } @@ -41,7 +41,7 @@ public void testGetValueAggregatorForCountType() { public void testGetValueAggregatorForAvgType() { assertThrows( IllegalStateException.class, - () -> ValueAggregatorFactory.getValueAggregator(MetricStat.AVG, StarTreeNumericType.LONG) + () -> ValueAggregatorFactory.getValueAggregator(MetricStat.AVG, NumberFieldMapper.NumberType.LONG) ); } diff --git a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/builder/AbstractStarTreeBuilderTests.java b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/builder/AbstractStarTreeBuilderTests.java index 6e23e5eacaecd..ad54bda9a916e 100644 --- a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/builder/AbstractStarTreeBuilderTests.java +++ b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/builder/AbstractStarTreeBuilderTests.java @@ -47,7 +47,6 @@ import org.opensearch.index.compositeindex.datacube.startree.StarTreeField; import org.opensearch.index.compositeindex.datacube.startree.StarTreeFieldConfiguration; import org.opensearch.index.compositeindex.datacube.startree.StarTreeTestUtils; -import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType; import org.opensearch.index.compositeindex.datacube.startree.fileformats.meta.StarTreeMetadata; import org.opensearch.index.compositeindex.datacube.startree.index.StarTreeValues; import org.opensearch.index.compositeindex.datacube.startree.node.InMemoryTreeNode; @@ -56,6 +55,7 @@ import org.opensearch.index.compositeindex.datacube.startree.utils.StarTreeUtils; import org.opensearch.index.mapper.ContentPath; import org.opensearch.index.mapper.DocumentMapper; +import org.opensearch.index.mapper.FieldValueConverter; import org.opensearch.index.mapper.Mapper; import org.opensearch.index.mapper.MapperService; import org.opensearch.index.mapper.MappingLookup; @@ -1895,13 +1895,13 @@ private void validateStarTreeFileFormats( StarTreeValues starTreeValues = new StarTreeValues(expectedStarTreeMetadata, dataIn, compositeDocValuesProducer, readState); assertEquals(expectedStarTreeMetadata.getStarTreeDocCount(), starTreeValues.getStarTreeDocumentCount()); - List starTreeNumericTypes = new ArrayList<>(); + List fieldValueConverters = new ArrayList<>(); builder.metricAggregatorInfos.forEach( - metricAggregatorInfo -> starTreeNumericTypes.add(metricAggregatorInfo.getValueAggregators().getAggregatedValueType()) + metricAggregatorInfo -> fieldValueConverters.add(metricAggregatorInfo.getValueAggregators().getAggregatedValueType()) ); StarTreeDocument[] starTreeDocuments = StarTreeTestUtils.getSegmentsStarTreeDocuments( List.of(starTreeValues), - starTreeNumericTypes, + fieldValueConverters, readState.segmentInfo.maxDoc() ); diff --git a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/builder/BaseStarTreeBuilderTests.java b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/builder/BaseStarTreeBuilderTests.java index 22a0fb7875714..8f750b09c75ee 100644 --- a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/builder/BaseStarTreeBuilderTests.java +++ b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/builder/BaseStarTreeBuilderTests.java @@ -36,7 +36,6 @@ import org.opensearch.index.compositeindex.datacube.startree.aggregators.MetricAggregatorInfo; import org.opensearch.index.compositeindex.datacube.startree.index.StarTreeValues; import org.opensearch.index.compositeindex.datacube.startree.utils.SequentialDocValuesIterator; -import org.opensearch.index.fielddata.IndexNumericFieldData; import org.opensearch.index.mapper.ContentPath; import org.opensearch.index.mapper.DocumentMapper; import org.opensearch.index.mapper.Mapper; @@ -229,8 +228,8 @@ Iterator mergeStarTrees(List starTreeValues) t public void test_generateMetricAggregatorInfos() throws IOException { List metricAggregatorInfos = builder.generateMetricAggregatorInfos(mapperService); List expectedMetricAggregatorInfos = List.of( - new MetricAggregatorInfo(MetricStat.SUM, "field2", starTreeField.getName(), IndexNumericFieldData.NumericType.DOUBLE), - new MetricAggregatorInfo(MetricStat.SUM, "field4", starTreeField.getName(), IndexNumericFieldData.NumericType.DOUBLE) + new MetricAggregatorInfo(MetricStat.SUM, "field2", starTreeField.getName(), NumberFieldMapper.NumberType.DOUBLE), + new MetricAggregatorInfo(MetricStat.SUM, "field4", starTreeField.getName(), NumberFieldMapper.NumberType.DOUBLE) ); assertEquals(metricAggregatorInfos, expectedMetricAggregatorInfos); } diff --git a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/fileformats/meta/StarTreeMetadataTests.java b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/fileformats/meta/StarTreeMetadataTests.java index 6fac5362c3dbb..2127eeb9636d3 100644 --- a/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/fileformats/meta/StarTreeMetadataTests.java +++ b/server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/fileformats/meta/StarTreeMetadataTests.java @@ -31,8 +31,8 @@ import org.opensearch.index.compositeindex.datacube.startree.StarTreeFieldConfiguration; import org.opensearch.index.compositeindex.datacube.startree.aggregators.MetricAggregatorInfo; import org.opensearch.index.compositeindex.datacube.startree.fileformats.StarTreeWriter; -import org.opensearch.index.fielddata.IndexNumericFieldData; import org.opensearch.index.mapper.CompositeMappedFieldType; +import org.opensearch.index.mapper.NumberFieldMapper; import org.opensearch.test.OpenSearchTestCase; import org.junit.Before; @@ -137,7 +137,7 @@ public void test_starTreeMetadata() throws IOException { metricType, metric.getField(), starTreeField.getName(), - IndexNumericFieldData.NumericType.DOUBLE + NumberFieldMapper.NumberType.DOUBLE ); metricAggregatorInfos.add(metricAggregatorInfo); } diff --git a/server/src/test/java/org/opensearch/index/mapper/ObjectMapperTests.java b/server/src/test/java/org/opensearch/index/mapper/ObjectMapperTests.java index ff0533de4fe8d..cb06bf23d9cbe 100644 --- a/server/src/test/java/org/opensearch/index/mapper/ObjectMapperTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/ObjectMapperTests.java @@ -504,7 +504,7 @@ public void testCompositeFields() throws Exception { .startObject("config") .startArray("ordered_dimensions") .startObject() - .field("name", "@timestamp") + .field("name", "node") .endObject() .startObject() .field("name", "status") @@ -522,8 +522,8 @@ public void testCompositeFields() throws Exception { .endObject() .endObject() .startObject("properties") - .startObject("@timestamp") - .field("type", "date") + .startObject("node") + .field("type", "integer") .endObject() .startObject("status") .field("type", "integer") @@ -561,9 +561,9 @@ public void testCompositeFields() throws Exception { StarTreeMapper starTreeMapper = (StarTreeMapper) mapper; assertEquals("star_tree", starTreeMapper.fieldType().typeName()); // Check that field in properties was parsed correctly as well - mapper = documentMapper.root().getMapper("@timestamp"); + mapper = documentMapper.root().getMapper("node"); assertNotNull(mapper); - assertEquals("date", mapper.typeName()); + assertEquals("integer", mapper.typeName()); FeatureFlags.initializeFeatureFlags(Settings.EMPTY); } diff --git a/server/src/test/java/org/opensearch/index/mapper/StarTreeMapperTests.java b/server/src/test/java/org/opensearch/index/mapper/StarTreeMapperTests.java index e06d9889ec905..daa9fda7a5a3b 100644 --- a/server/src/test/java/org/opensearch/index/mapper/StarTreeMapperTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/StarTreeMapperTests.java @@ -74,14 +74,7 @@ public void testValidStarTree() throws IOException { for (CompositeMappedFieldType type : compositeFieldTypes) { StarTreeMapper.StarTreeFieldType starTreeFieldType = (StarTreeMapper.StarTreeFieldType) type; assertEquals(2, starTreeFieldType.getDimensions().size()); - assertEquals("@timestamp", starTreeFieldType.getDimensions().get(0).getField()); - assertTrue(starTreeFieldType.getDimensions().get(0) instanceof DateDimension); - DateDimension dateDim = (DateDimension) starTreeFieldType.getDimensions().get(0); - List expectedTimeUnits = Arrays.asList( - Rounding.DateTimeUnit.DAY_OF_MONTH, - Rounding.DateTimeUnit.MONTH_OF_YEAR - ); - assertEquals(expectedTimeUnits, dateDim.getIntervals()); + assertEquals("node", starTreeFieldType.getDimensions().get(0).getField()); assertEquals("status", starTreeFieldType.getDimensions().get(1).getField()); assertEquals(2, starTreeFieldType.getMetrics().size()); assertEquals("size", starTreeFieldType.getMetrics().get(0).getField()); @@ -92,7 +85,7 @@ public void testValidStarTree() throws IOException { assertEquals(100, starTreeFieldType.getStarTreeConfig().maxLeafDocs()); assertEquals(StarTreeFieldConfiguration.StarTreeBuildMode.OFF_HEAP, starTreeFieldType.getStarTreeConfig().getBuildMode()); assertEquals( - new HashSet<>(Arrays.asList("@timestamp", "status")), + new HashSet<>(Arrays.asList("node", "status")), starTreeFieldType.getStarTreeConfig().getSkipStarNodeCreationInDims() ); } @@ -137,14 +130,7 @@ public void testMetricsWithJustSum() throws IOException { Set compositeFieldTypes = mapperService.getCompositeFieldTypes(); for (CompositeMappedFieldType type : compositeFieldTypes) { StarTreeMapper.StarTreeFieldType starTreeFieldType = (StarTreeMapper.StarTreeFieldType) type; - assertEquals("@timestamp", starTreeFieldType.getDimensions().get(0).getField()); - assertTrue(starTreeFieldType.getDimensions().get(0) instanceof DateDimension); - DateDimension dateDim = (DateDimension) starTreeFieldType.getDimensions().get(0); - List expectedTimeUnits = Arrays.asList( - Rounding.DateTimeUnit.DAY_OF_MONTH, - Rounding.DateTimeUnit.MONTH_OF_YEAR - ); - assertEquals(expectedTimeUnits, dateDim.getIntervals()); + assertEquals("node", starTreeFieldType.getDimensions().get(0).getField()); assertEquals("status", starTreeFieldType.getDimensions().get(1).getField()); assertEquals("size", starTreeFieldType.getMetrics().get(0).getField()); @@ -154,7 +140,7 @@ public void testMetricsWithJustSum() throws IOException { assertEquals(100, starTreeFieldType.getStarTreeConfig().maxLeafDocs()); assertEquals(StarTreeFieldConfiguration.StarTreeBuildMode.OFF_HEAP, starTreeFieldType.getStarTreeConfig().getBuildMode()); assertEquals( - new HashSet<>(Arrays.asList("@timestamp", "status")), + new HashSet<>(Arrays.asList("node", "status")), starTreeFieldType.getStarTreeConfig().getSkipStarNodeCreationInDims() ); } @@ -165,14 +151,7 @@ public void testMetricsWithCountAndSum() throws IOException { Set compositeFieldTypes = mapperService.getCompositeFieldTypes(); for (CompositeMappedFieldType type : compositeFieldTypes) { StarTreeMapper.StarTreeFieldType starTreeFieldType = (StarTreeMapper.StarTreeFieldType) type; - assertEquals("@timestamp", starTreeFieldType.getDimensions().get(0).getField()); - assertTrue(starTreeFieldType.getDimensions().get(0) instanceof DateDimension); - DateDimension dateDim = (DateDimension) starTreeFieldType.getDimensions().get(0); - List expectedTimeUnits = Arrays.asList( - Rounding.DateTimeUnit.DAY_OF_MONTH, - Rounding.DateTimeUnit.MONTH_OF_YEAR - ); - assertEquals(expectedTimeUnits, dateDim.getIntervals()); + assertEquals("node", starTreeFieldType.getDimensions().get(0).getField()); assertEquals("status", starTreeFieldType.getDimensions().get(1).getField()); assertEquals("size", starTreeFieldType.getMetrics().get(0).getField()); @@ -187,7 +166,7 @@ public void testMetricsWithCountAndSum() throws IOException { assertEquals(100, starTreeFieldType.getStarTreeConfig().maxLeafDocs()); assertEquals(StarTreeFieldConfiguration.StarTreeBuildMode.OFF_HEAP, starTreeFieldType.getStarTreeConfig().getBuildMode()); assertEquals( - new HashSet<>(Arrays.asList("@timestamp", "status")), + new HashSet<>(Arrays.asList("node", "status")), starTreeFieldType.getStarTreeConfig().getSkipStarNodeCreationInDims() ); } @@ -198,14 +177,7 @@ public void testValidStarTreeDefaults() throws IOException { Set compositeFieldTypes = mapperService.getCompositeFieldTypes(); for (CompositeMappedFieldType type : compositeFieldTypes) { StarTreeMapper.StarTreeFieldType starTreeFieldType = (StarTreeMapper.StarTreeFieldType) type; - assertEquals("@timestamp", starTreeFieldType.getDimensions().get(0).getField()); - assertTrue(starTreeFieldType.getDimensions().get(0) instanceof DateDimension); - DateDimension dateDim = (DateDimension) starTreeFieldType.getDimensions().get(0); - List expectedTimeUnits = Arrays.asList( - Rounding.DateTimeUnit.MINUTES_OF_HOUR, - Rounding.DateTimeUnit.HOUR_OF_DAY - ); - assertEquals(expectedTimeUnits, dateDim.getIntervals()); + assertEquals("node", starTreeFieldType.getDimensions().get(0).getField()); assertEquals("status", starTreeFieldType.getDimensions().get(1).getField()); assertEquals(3, starTreeFieldType.getMetrics().size()); assertEquals("status", starTreeFieldType.getMetrics().get(0).getField()); @@ -278,7 +250,7 @@ public void testMissingDims() { MapperParsingException.class, () -> createMapperService(getMinMapping(false, false, true, false)) ); - assertThat(ex.getMessage(), containsString("Failed to parse mapping [_doc]: unknown dimension field [@timestamp]")); + assertThat(ex.getMessage(), containsString("Failed to parse mapping [_doc]: unknown dimension field [node]")); } public void testMissingMetrics() { @@ -314,7 +286,7 @@ public void testInvalidDimType() { () -> createMapperService(getInvalidMapping(false, false, true, false)) ); assertEquals( - "Failed to parse mapping [_doc]: unsupported field type associated with dimension [@timestamp] as part of star tree field [startree]", + "Failed to parse mapping [_doc]: unsupported field type associated with dimension [node] as part of star tree field [startree]", ex.getMessage() ); } @@ -511,7 +483,7 @@ public void testValidations() throws IOException { ) ); assertEquals( - "Aggregations not supported for the dimension field [@timestamp] with field type [date] as part of star tree field", + "Aggregations not supported for the dimension field [node] with field type [integer] as part of star tree field", ex.getMessage() ); @@ -534,17 +506,13 @@ private XContentBuilder getExpandedMappingWithJustAvg(String dim, String metric) b.field("max_leaf_docs", 100); b.startArray("skip_star_node_creation_for_dimensions"); { - b.value("@timestamp"); + b.value("node"); b.value("status"); } b.endArray(); b.startArray("ordered_dimensions"); b.startObject(); - b.field("name", "@timestamp"); - b.startArray("calendar_intervals"); - b.value("day"); - b.value("month"); - b.endArray(); + b.field("name", "node"); b.endObject(); b.startObject(); b.field("name", dim); @@ -562,8 +530,8 @@ private XContentBuilder getExpandedMappingWithJustAvg(String dim, String metric) b.endObject(); b.endObject(); b.startObject("properties"); - b.startObject("@timestamp"); - b.field("type", "date"); + b.startObject("node"); + b.field("type", "integer"); b.endObject(); b.startObject("status"); b.field("type", "integer"); @@ -585,7 +553,7 @@ private XContentBuilder getMappingWithDuplicateFields(boolean isDuplicateDim, bo .startObject("config") .startArray("ordered_dimensions") .startObject() - .field("name", "timestamp") + .field("name", "node") .endObject() .startObject() .field("name", "numeric_dv") @@ -606,8 +574,8 @@ private XContentBuilder getMappingWithDuplicateFields(boolean isDuplicateDim, bo .endObject() .endObject() .startObject("properties") - .startObject("timestamp") - .field("type", "date") + .startObject("node") + .field("type", "integer") .endObject() .startObject("numeric_dv") .field("type", "integer") @@ -634,17 +602,13 @@ private XContentBuilder getExpandedMappingWithJustSum(String dim, String metric) b.field("max_leaf_docs", 100); b.startArray("skip_star_node_creation_for_dimensions"); { - b.value("@timestamp"); + b.value("node"); b.value("status"); } b.endArray(); b.startArray("ordered_dimensions"); b.startObject(); - b.field("name", "@timestamp"); - b.startArray("calendar_intervals"); - b.value("day"); - b.value("month"); - b.endArray(); + b.field("name", "node"); b.endObject(); b.startObject(); b.field("name", dim); @@ -662,8 +626,8 @@ private XContentBuilder getExpandedMappingWithJustSum(String dim, String metric) b.endObject(); b.endObject(); b.startObject("properties"); - b.startObject("@timestamp"); - b.field("type", "date"); + b.startObject("node"); + b.field("type", "integer"); b.endObject(); b.startObject("status"); b.field("type", "integer"); @@ -684,17 +648,13 @@ private XContentBuilder getExpandedMappingWithSumAndCount(String dim, String met b.field("max_leaf_docs", 100); b.startArray("skip_star_node_creation_for_dimensions"); { - b.value("@timestamp"); + b.value("node"); b.value("status"); } b.endArray(); b.startArray("ordered_dimensions"); b.startObject(); - b.field("name", "@timestamp"); - b.startArray("calendar_intervals"); - b.value("day"); - b.value("month"); - b.endArray(); + b.field("name", "node"); b.endObject(); b.startObject(); b.field("name", dim); @@ -713,8 +673,8 @@ private XContentBuilder getExpandedMappingWithSumAndCount(String dim, String met b.endObject(); b.endObject(); b.startObject("properties"); - b.startObject("@timestamp"); - b.field("type", "date"); + b.startObject("node"); + b.field("type", "integer"); b.endObject(); b.startObject("status"); b.field("type", "integer"); @@ -740,7 +700,7 @@ private XContentBuilder getMinMapping(boolean isEmptyDims, boolean isEmptyMetric if (!isEmptyDims) { b.startArray("ordered_dimensions"); b.startObject(); - b.field("name", "@timestamp"); + b.field("name", "node"); b.endObject(); b.startObject(); b.field("name", "status"); @@ -762,8 +722,8 @@ private XContentBuilder getMinMapping(boolean isEmptyDims, boolean isEmptyMetric b.endObject(); b.startObject("properties"); if (!missingDim) { - b.startObject("@timestamp"); - b.field("type", "date"); + b.startObject("node"); + b.field("type", "integer"); b.endObject(); } b.startObject("status"); @@ -876,7 +836,7 @@ private XContentBuilder getInvalidMapping( b.startArray("ordered_dimensions"); if (!singleDim) { b.startObject(); - b.field("name", "@timestamp"); + b.field("name", "node"); b.endObject(); } b.startObject(); @@ -901,9 +861,9 @@ private XContentBuilder getInvalidMapping( b.endObject(); b.endObject(); b.startObject("properties"); - b.startObject("@timestamp"); + b.startObject("node"); if (!invalidDimType) { - b.field("type", "date"); + b.field("type", "integer"); } else { b.field("type", "keyword"); } @@ -946,7 +906,7 @@ private XContentBuilder getInvalidMappingWithDv( b.startArray("ordered_dimensions"); if (!singleDim) { b.startObject(); - b.field("name", "@timestamp"); + b.field("name", "node"); b.endObject(); } b.startObject(); @@ -965,12 +925,12 @@ private XContentBuilder getInvalidMappingWithDv( b.endObject(); b.endObject(); b.startObject("properties"); - b.startObject("@timestamp"); + b.startObject("node"); if (!invalidDimType) { - b.field("type", "date"); + b.field("type", "integer"); b.field("doc_values", "true"); } else { - b.field("type", "date"); + b.field("type", "integer"); b.field("doc_values", "false"); } b.endObject(); From d28fbc3bc2a4fbf0a70558e775e3f7a9e15d2617 Mon Sep 17 00:00:00 2001 From: gargharsh3134 <51459091+gargharsh3134@users.noreply.github.com> Date: Wed, 4 Sep 2024 16:11:12 +0530 Subject: [PATCH 4/8] =?UTF-8?q?Optimizing=20get=20indices=20to=20segments?= =?UTF-8?q?=20map=20calculations=20for=20IndicesSegmen=E2=80=A6=20(#14717)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Optimizing get indices to segments map calculations for IndicesSegmentResponse class Signed-off-by: Harsh Garg --- .../segments/IndicesSegmentResponse.java | 30 ++++++----- .../segments/IndicesSegmentResponseTests.java | 53 +++++++++++++++++++ 2 files changed, 69 insertions(+), 14 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/admin/indices/segments/IndicesSegmentResponse.java b/server/src/main/java/org/opensearch/action/admin/indices/segments/IndicesSegmentResponse.java index 648f58dada4f9..b69d4b6f624b8 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/segments/IndicesSegmentResponse.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/segments/IndicesSegmentResponse.java @@ -45,13 +45,12 @@ import org.opensearch.index.engine.Segment; import java.io.IOException; -import java.util.ArrayList; +import java.util.Arrays; +import java.util.Comparator; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.Set; /** * Transport response for retrieving indices segment information @@ -86,21 +85,24 @@ public Map getIndices() { return indicesSegments; } Map indicesSegments = new HashMap<>(); - - Set indices = new HashSet<>(); - for (ShardSegments shard : shards) { - indices.add(shard.getShardRouting().getIndexName()); + if (shards.length == 0) { + this.indicesSegments = indicesSegments; + return indicesSegments; } - for (String indexName : indices) { - List shards = new ArrayList<>(); - for (ShardSegments shard : this.shards) { - if (shard.getShardRouting().getIndexName().equals(indexName)) { - shards.add(shard); - } + Arrays.sort(shards, Comparator.comparing(shardSegment -> shardSegment.getShardRouting().getIndexName())); + int startIndexPos = 0; + String startIndexName = shards[startIndexPos].getShardRouting().getIndexName(); + for (int i = 0; i < shards.length; i++) { + if (!shards[i].getShardRouting().getIndexName().equals(startIndexName)) { + indicesSegments.put(startIndexName, new IndexSegments(startIndexName, Arrays.copyOfRange(shards, startIndexPos, i))); + startIndexPos = i; + startIndexName = shards[startIndexPos].getShardRouting().getIndexName(); } - indicesSegments.put(indexName, new IndexSegments(indexName, shards.toArray(new ShardSegments[0]))); } + // Add the last shardSegment from shards list which would have got missed in the loop above + indicesSegments.put(startIndexName, new IndexSegments(startIndexName, Arrays.copyOfRange(shards, startIndexPos, shards.length))); + this.indicesSegments = indicesSegments; return indicesSegments; } diff --git a/server/src/test/java/org/opensearch/action/admin/indices/segments/IndicesSegmentResponseTests.java b/server/src/test/java/org/opensearch/action/admin/indices/segments/IndicesSegmentResponseTests.java index b90be87e3a600..c01335eb38afc 100644 --- a/server/src/test/java/org/opensearch/action/admin/indices/segments/IndicesSegmentResponseTests.java +++ b/server/src/test/java/org/opensearch/action/admin/indices/segments/IndicesSegmentResponseTests.java @@ -42,7 +42,10 @@ import org.opensearch.index.engine.Segment; import org.opensearch.test.OpenSearchTestCase; +import java.util.ArrayList; import java.util.Collections; +import java.util.List; +import java.util.Map; import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder; @@ -68,4 +71,54 @@ public void testToXContentSerialiationWithSortedFields() throws Exception { response.toXContent(builder, ToXContent.EMPTY_PARAMS); } } + + public void testGetIndices() { + final int totalIndices = 5; + final int shardsPerIndex = 3; + final int segmentsPerShard = 2; + // Preparing a ShardSegments list, which will have (totalIndices * shardsPerIndex) shardSegments. + // Indices will be named -> foo1, foo2, ..., foo{totalIndices} + List shardSegmentsList = new ArrayList<>(); + for (int indexName = 0; indexName < totalIndices; indexName++) { + for (int shardId = 0; shardId < shardsPerIndex; shardId++) { + ShardRouting shardRouting = TestShardRouting.newShardRouting( + "foo" + indexName, + shardId, + "node_id", + true, + ShardRoutingState.STARTED + ); + List segmentList = new ArrayList<>(); + for (int segmentNum = 0; segmentNum < segmentsPerShard; segmentNum++) { + segmentList.add(new Segment("foo" + indexName + shardId + segmentNum)); + } + shardSegmentsList.add(new ShardSegments(shardRouting, segmentList)); + } + } + Collections.shuffle(shardSegmentsList, random()); + + // Prepare the IndicesSegmentResponse object and get the indicesSegments map + IndicesSegmentResponse response = new IndicesSegmentResponse( + shardSegmentsList.toArray(new ShardSegments[0]), + totalIndices * shardsPerIndex, + totalIndices * shardsPerIndex, + 0, + Collections.emptyList() + ); + Map indicesSegments = response.getIndices(); + + assertEquals(totalIndices, indicesSegments.size()); + for (Map.Entry indexSegmentEntry : indicesSegments.entrySet()) { + assertEquals(shardsPerIndex, indexSegmentEntry.getValue().getShards().size()); + for (IndexShardSegments indexShardSegment : indexSegmentEntry.getValue().getShards().values()) { + for (ShardSegments shardSegment : indexShardSegment.getShards()) { + assertEquals(segmentsPerShard, shardSegment.getSegments().size()); + for (int i = 0; i < segmentsPerShard; i++) { + String segmentName = indexSegmentEntry.getKey() + shardSegment.getShardRouting().getId() + i; + assertEquals(segmentName, shardSegment.getSegments().get(i).getName()); + } + } + } + } + } } From 2793634f98819029e3bee327e1a47a89f8acc580 Mon Sep 17 00:00:00 2001 From: rajiv-kv <157019998+rajiv-kv@users.noreply.github.com> Date: Wed, 4 Sep 2024 16:35:00 +0530 Subject: [PATCH 5/8] relaxing the join validation for nodes which have only store disabled but only publication enabled (#15471) Signed-off-by: Rajiv Kumar Vaidyanathan --- CHANGELOG.md | 1 + .../coordination/JoinTaskExecutor.java | 45 ++++- .../metadata/RepositoriesMetadata.java | 18 ++ .../cluster/node/DiscoveryNode.java | 4 +- .../remotestore/RemoteStoreNodeAttribute.java | 13 ++ ...ransportClusterManagerNodeActionTests.java | 2 + .../coordination/JoinTaskExecutorTests.java | 157 +++++++++++++++--- .../MetadataCreateIndexServiceTests.java | 9 +- .../allocation/FailedShardsRoutingTests.java | 3 + ...eStoreMigrationAllocationDeciderTests.java | 2 + .../index/remote/RemoteStoreUtilsTests.java | 2 + .../index/shard/IndexShardTestUtils.java | 2 + 12 files changed, 235 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e255ad694206..f67e9a4c257f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add canRemain method to TargetPoolAllocationDecider to move shards from local to remote pool for hot to warm tiering ([#15010](https://github.com/opensearch-project/OpenSearch/pull/15010)) - ClusterManagerTaskThrottler Improvements ([#15508](https://github.com/opensearch-project/OpenSearch/pull/15508)) - Reset DiscoveryNodes in all transport node actions request ([#15131](https://github.com/opensearch-project/OpenSearch/pull/15131)) +- Relax the join validation for Remote State publication ([#15471](https://github.com/opensearch-project/OpenSearch/pull/15471)) ### Dependencies - Bump `netty` from 4.1.111.Final to 4.1.112.Final ([#15081](https://github.com/opensearch-project/OpenSearch/pull/15081)) diff --git a/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java b/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java index f77a7ffc8ce8e..838b5723b217b 100644 --- a/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java +++ b/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java @@ -67,6 +67,7 @@ import static org.opensearch.cluster.decommission.DecommissionHelper.nodeCommissioned; import static org.opensearch.gateway.GatewayService.STATE_NOT_RECOVERED_BLOCK; +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_CLUSTER_PUBLICATION_REPO_NAME_ATTRIBUTES; import static org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode; import static org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode.MIXED; import static org.opensearch.node.remotestore.RemoteStoreNodeService.CompatibilityMode.STRICT; @@ -458,7 +459,7 @@ public static void ensureNodesCompatibility( ); } - ensureRemoteStoreNodesCompatibility(joiningNode, currentNodes, metadata); + ensureRemoteRepositoryCompatibility(joiningNode, currentNodes, metadata); } /** @@ -491,6 +492,30 @@ public static void ensureNodeCommissioned(DiscoveryNode node, Metadata metadata) } } + public static void ensureRemoteRepositoryCompatibility(DiscoveryNode joiningNode, DiscoveryNodes currentNodes, Metadata metadata) { + List existingNodes = new ArrayList<>(currentNodes.getNodes().values()); + + boolean isClusterRemoteStoreEnabled = existingNodes.stream().anyMatch(DiscoveryNode::isRemoteStoreNode); + if (isClusterRemoteStoreEnabled || joiningNode.isRemoteStoreNode()) { + ensureRemoteStoreNodesCompatibility(joiningNode, currentNodes, metadata); + } else { + ensureRemoteClusterStateNodesCompatibility(joiningNode, currentNodes); + } + } + + private static void ensureRemoteClusterStateNodesCompatibility(DiscoveryNode joiningNode, DiscoveryNodes currentNodes) { + List existingNodes = new ArrayList<>(currentNodes.getNodes().values()); + + assert existingNodes.isEmpty() == false; + Optional remotePublicationNode = existingNodes.stream() + .filter(DiscoveryNode::isRemoteStatePublicationEnabled) + .findFirst(); + + if (remotePublicationNode.isPresent() && joiningNode.isRemoteStatePublicationEnabled()) { + ensureRepositoryCompatibility(joiningNode, remotePublicationNode.get(), REMOTE_CLUSTER_PUBLICATION_REPO_NAME_ATTRIBUTES); + } + } + /** * The method ensures homogeneity - * 1. The joining node has to be a remote store backed if it's joining a remote store backed cluster. Validates @@ -506,6 +531,7 @@ public static void ensureNodeCommissioned(DiscoveryNode node, Metadata metadata) * needs to be modified. */ private static void ensureRemoteStoreNodesCompatibility(DiscoveryNode joiningNode, DiscoveryNodes currentNodes, Metadata metadata) { + List existingNodes = new ArrayList<>(currentNodes.getNodes().values()); assert existingNodes.isEmpty() == false; @@ -587,6 +613,23 @@ private static void ensureRemoteStoreNodesCompatibility( } } + private static void ensureRepositoryCompatibility(DiscoveryNode joiningNode, DiscoveryNode existingNode, List reposToValidate) { + + RemoteStoreNodeAttribute joiningRemoteStoreNodeAttribute = new RemoteStoreNodeAttribute(joiningNode); + RemoteStoreNodeAttribute existingRemoteStoreNodeAttribute = new RemoteStoreNodeAttribute(existingNode); + + if (existingRemoteStoreNodeAttribute.equalsForRepositories(joiningRemoteStoreNodeAttribute, reposToValidate) == false) { + throw new IllegalStateException( + "a remote store node [" + + joiningNode + + "] is trying to join a remote store cluster with incompatible node attributes in " + + "comparison with existing node [" + + existingNode + + "]" + ); + } + } + public static Collection> addBuiltInJoinValidators( Collection> onJoinValidators ) { diff --git a/server/src/main/java/org/opensearch/cluster/metadata/RepositoriesMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/RepositoriesMetadata.java index 4b3dc7964a87b..59452e33191d7 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/RepositoriesMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/RepositoriesMetadata.java @@ -184,6 +184,24 @@ public boolean equalsIgnoreGenerationsWithRepoSkip(@Nullable RepositoriesMetadat .filter(repo -> !reposToSkip.contains(repo.name())) .collect(Collectors.toList()); + return equalsRepository(currentRepositories, otherRepositories); + } + + public boolean equalsIgnoreGenerationsForRepo(@Nullable RepositoriesMetadata other, List reposToValidate) { + if (other == null) { + return false; + } + List currentRepositories = repositories.stream() + .filter(repo -> reposToValidate.contains(repo.name())) + .collect(Collectors.toList()); + List otherRepositories = other.repositories.stream() + .filter(repo -> reposToValidate.contains(repo.name())) + .collect(Collectors.toList()); + + return equalsRepository(currentRepositories, otherRepositories); + } + + public static boolean equalsRepository(List currentRepositories, List otherRepositories) { if (otherRepositories.size() != currentRepositories.size()) { return false; } diff --git a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java index 3ed7633dce189..a6f0a457f7f9b 100644 --- a/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java +++ b/server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java @@ -65,6 +65,7 @@ import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_NODE_ATTRIBUTE_KEY_PREFIX; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY; +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY; /** * A discovery node represents a node that is part of the cluster. @@ -509,7 +510,8 @@ public boolean isSearchNode() { * @return true if the node contains remote store node attributes, false otherwise */ public boolean isRemoteStoreNode() { - return this.getAttributes().keySet().stream().anyMatch(key -> key.startsWith(REMOTE_STORE_NODE_ATTRIBUTE_KEY_PREFIX)); + return this.getAttributes().keySet().stream().anyMatch(key -> key.equals(REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY)) + && this.getAttributes().keySet().stream().anyMatch(key -> key.equals(REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY)); } /** diff --git a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java index efaefd530b34d..55971398634c5 100644 --- a/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java +++ b/server/src/main/java/org/opensearch/node/remotestore/RemoteStoreNodeAttribute.java @@ -55,6 +55,11 @@ public class RemoteStoreNodeAttribute { REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY ); + public static List REMOTE_CLUSTER_PUBLICATION_REPO_NAME_ATTRIBUTES = List.of( + REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY, + REMOTE_STORE_ROUTING_TABLE_REPOSITORY_NAME_ATTRIBUTE_KEY + ); + /** * Creates a new {@link RemoteStoreNodeAttribute} */ @@ -261,6 +266,14 @@ public boolean equalsWithRepoSkip(Object o, List reposToSkip) { return this.getRepositoriesMetadata().equalsIgnoreGenerationsWithRepoSkip(that.getRepositoriesMetadata(), reposToSkip); } + public boolean equalsForRepositories(Object otherNode, List repositoryToValidate) { + if (this == otherNode) return true; + if (otherNode == null || getClass() != otherNode.getClass()) return false; + + RemoteStoreNodeAttribute other = (RemoteStoreNodeAttribute) otherNode; + return this.getRepositoriesMetadata().equalsIgnoreGenerationsForRepo(other.repositoriesMetadata, repositoryToValidate); + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/server/src/test/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeActionTests.java b/server/src/test/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeActionTests.java index 37e884502b613..3c1c84653b384 100644 --- a/server/src/test/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeActionTests.java +++ b/server/src/test/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeActionTests.java @@ -85,6 +85,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import static org.opensearch.index.remote.RemoteMigrationIndexMetadataUpdaterTests.createIndexMetadataWithRemoteStoreSettings; +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING; @@ -901,6 +902,7 @@ public void testDontAllowSwitchingCompatibilityModeForClusterWithMultipleVersion private Map getRemoteStoreNodeAttributes() { Map remoteStoreNodeAttributes = new HashMap<>(); + remoteStoreNodeAttributes.put(REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY, "my-cluster-repo-1"); remoteStoreNodeAttributes.put(REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, "my-segment-repo-1"); remoteStoreNodeAttributes.put(REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY, "my-translog-repo-1"); return remoteStoreNodeAttributes; diff --git a/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java b/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java index 9f463673aa6a6..d192a2556c36b 100644 --- a/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java +++ b/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java @@ -611,6 +611,100 @@ public void testJoinClusterWithRemoteStateNodeJoiningRemoteStateCluster() { JoinTaskExecutor.ensureNodesCompatibility(joiningNode, currentState.getNodes(), currentState.metadata()); } + public void testJoinRemotePublicationClusterWithNonRemoteNodes() { + final DiscoveryNode existingNode = new DiscoveryNode( + UUIDs.base64UUID(), + buildNewFakeTransportAddress(), + remotePublicationNodeAttributes(), + DiscoveryNodeRole.BUILT_IN_ROLES, + Version.CURRENT + ); + ClusterState currentState = ClusterState.builder(ClusterName.DEFAULT) + .nodes(DiscoveryNodes.builder().add(existingNode).localNodeId(existingNode.getId()).build()) + .build(); + + DiscoveryNode joiningNode = newDiscoveryNode(new HashMap<>()); + JoinTaskExecutor.ensureNodesCompatibility(joiningNode, currentState.getNodes(), currentState.metadata()); + } + + public void testJoinRemotePublicationCluster() { + final DiscoveryNode existingNode = new DiscoveryNode( + UUIDs.base64UUID(), + buildNewFakeTransportAddress(), + remotePublicationNodeAttributes(), + DiscoveryNodeRole.BUILT_IN_ROLES, + Version.CURRENT + ); + ClusterState currentState = ClusterState.builder(ClusterName.DEFAULT) + .nodes(DiscoveryNodes.builder().add(existingNode).localNodeId(existingNode.getId()).build()) + .build(); + + DiscoveryNode joiningNode = newDiscoveryNode(remotePublicationNodeAttributes()); + JoinTaskExecutor.ensureNodesCompatibility(joiningNode, currentState.getNodes(), currentState.metadata()); + } + + public void testJoinRemotePubClusterWithRemoteStoreNodes() { + final DiscoveryNode existingNode = new DiscoveryNode( + UUIDs.base64UUID(), + buildNewFakeTransportAddress(), + remotePublicationNodeAttributes(), + DiscoveryNodeRole.BUILT_IN_ROLES, + Version.CURRENT + ); + ClusterState currentState = ClusterState.builder(ClusterName.DEFAULT) + .nodes(DiscoveryNodes.builder().add(existingNode).localNodeId(existingNode.getId()).build()) + .build(); + + Map newNodeAttributes = new HashMap<>(); + newNodeAttributes.putAll(remoteStateNodeAttributes(CLUSTER_STATE_REPO)); + newNodeAttributes.putAll(remoteRoutingTableAttributes(ROUTING_TABLE_REPO)); + newNodeAttributes.putAll(remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO)); + + DiscoveryNode joiningNode = newDiscoveryNode(newNodeAttributes); + Exception e = assertThrows( + IllegalStateException.class, + () -> JoinTaskExecutor.ensureNodesCompatibility(joiningNode, currentState.getNodes(), currentState.metadata()) + ); + assertTrue(e.getMessage().equals("a remote store node [" + joiningNode + "] is trying to join a non remote store cluster")); + } + + public void testPreventJoinRemotePublicationClusterWithIncompatibleAttributes() { + Map existingNodeAttributes = remotePublicationNodeAttributes(); + Map remoteStoreNodeAttributes = remotePublicationNodeAttributes(); + final DiscoveryNode existingNode = new DiscoveryNode( + UUIDs.base64UUID(), + buildNewFakeTransportAddress(), + existingNodeAttributes, + DiscoveryNodeRole.BUILT_IN_ROLES, + Version.CURRENT + ); + ClusterState currentState = ClusterState.builder(ClusterName.DEFAULT) + .nodes(DiscoveryNodes.builder().add(existingNode).localNodeId(existingNode.getId()).build()) + .build(); + + for (Map.Entry nodeAttribute : existingNodeAttributes.entrySet()) { + remoteStoreNodeAttributes.put(nodeAttribute.getKey(), null); + DiscoveryNode joiningNode = newDiscoveryNode(remoteStoreNodeAttributes); + Exception e = assertThrows( + IllegalStateException.class, + () -> JoinTaskExecutor.ensureNodesCompatibility(joiningNode, currentState.getNodes(), currentState.metadata()) + ); + assertTrue( + e.getMessage().equals("joining node [" + joiningNode + "] doesn't have the node attribute [" + nodeAttribute.getKey() + "]") + || e.getMessage() + .equals( + "a remote store node [" + + joiningNode + + "] is trying to join a remote store cluster with incompatible node attributes in comparison with existing node [" + + currentState.getNodes().getNodes().values().stream().findFirst().get() + + "]" + ) + ); + + remoteStoreNodeAttributes.put(nodeAttribute.getKey(), nodeAttribute.getValue()); + } + } + public void testPreventJoinClusterWithRemoteStateNodeJoiningRemoteStoreCluster() { Map existingNodeAttributes = remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO); final DiscoveryNode existingNode = new DiscoveryNode( @@ -628,16 +722,7 @@ public void testPreventJoinClusterWithRemoteStateNodeJoiningRemoteStoreCluster() IllegalStateException.class, () -> JoinTaskExecutor.ensureNodesCompatibility(joiningNode, currentState.getNodes(), currentState.metadata()) ); - assertTrue( - e.getMessage() - .equals( - "a remote store node [" - + joiningNode - + "] is trying to join a remote store cluster with incompatible node attributes in comparison with existing node [" - + currentState.getNodes().getNodes().values().stream().findFirst().get() - + "]" - ) - ); + assertTrue(e.getMessage().equals("a non remote store node [" + joiningNode + "] is trying to join a remote store cluster")); } public void testPreventJoinClusterWithRemoteStoreNodeJoiningRemoteStateCluster() { @@ -657,16 +742,7 @@ public void testPreventJoinClusterWithRemoteStoreNodeJoiningRemoteStateCluster() IllegalStateException.class, () -> JoinTaskExecutor.ensureNodesCompatibility(joiningNode, currentState.getNodes(), currentState.metadata()) ); - assertTrue( - e.getMessage() - .equals( - "a remote store node [" - + joiningNode - + "] is trying to join a remote store cluster with incompatible node attributes in comparison with existing node [" - + currentState.getNodes().getNodes().values().stream().findFirst().get() - + "]" - ) - ); + assertTrue(e.getMessage().equals("a remote store node [" + joiningNode + "] is trying to join a non remote store cluster")); } public void testUpdatesClusterStateWithSingleNodeCluster() throws Exception { @@ -1077,6 +1153,39 @@ public void testRemoteRoutingTableNodeJoinNodeWithRemoteAndRoutingRepoDifference JoinTaskExecutor.ensureNodesCompatibility(joiningNode, currentState.getNodes(), currentState.metadata()); } + public void testJoinRemoteStoreClusterWithRemotePublicationNodeInMixedMode() { + final DiscoveryNode remoteStoreNode = new DiscoveryNode( + UUIDs.base64UUID(), + buildNewFakeTransportAddress(), + remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO), + DiscoveryNodeRole.BUILT_IN_ROLES, + Version.CURRENT + ); + + final DiscoveryNode nonRemoteStoreNode = new DiscoveryNode( + UUIDs.base64UUID(), + buildNewFakeTransportAddress(), + new HashMap<>(), + DiscoveryNodeRole.BUILT_IN_ROLES, + Version.CURRENT + ); + + final Settings settings = Settings.builder() + .put(MIGRATION_DIRECTION_SETTING.getKey(), RemoteStoreNodeService.Direction.REMOTE_STORE) + .put(REMOTE_STORE_COMPATIBILITY_MODE_SETTING.getKey(), "mixed") + .build(); + final Settings nodeSettings = Settings.builder().put(REMOTE_STORE_MIGRATION_EXPERIMENTAL, "true").build(); + FeatureFlags.initializeFeatureFlags(nodeSettings); + Metadata metadata = Metadata.builder().persistentSettings(settings).build(); + ClusterState currentState = ClusterState.builder(ClusterName.DEFAULT) + .nodes(DiscoveryNodes.builder().add(remoteStoreNode).add(nonRemoteStoreNode).localNodeId(remoteStoreNode.getId()).build()) + .metadata(metadata) + .build(); + + DiscoveryNode joiningNode = newDiscoveryNode(remotePublicationNodeAttributes()); + JoinTaskExecutor.ensureNodesCompatibility(joiningNode, currentState.getNodes(), currentState.metadata()); + } + private void validateRepositoryMetadata(ClusterState updatedState, DiscoveryNode existingNode, int expectedRepositories) throws Exception { @@ -1115,6 +1224,7 @@ private DiscoveryNode newDiscoveryNode(Map attributes) { } private static final String SEGMENT_REPO = "segment-repo"; + private static final String TRANSLOG_REPO = "translog-repo"; private static final String CLUSTER_STATE_REPO = "cluster-state-repo"; private static final String COMMON_REPO = "remote-repo"; @@ -1161,6 +1271,13 @@ private Map remoteStoreNodeAttributes(String segmentRepoName, St }; } + private Map remotePublicationNodeAttributes() { + Map existingNodeAttributes = new HashMap<>(); + existingNodeAttributes.putAll(remoteStateNodeAttributes(CLUSTER_STATE_REPO)); + existingNodeAttributes.putAll(remoteRoutingTableAttributes(ROUTING_TABLE_REPO)); + return existingNodeAttributes; + } + private Map remoteStateNodeAttributes(String clusterStateRepo) { String clusterStateRepositoryTypeAttributeKey = String.format( Locale.getDefault(), diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java index 410c8a3d41940..d376d0db8d6f4 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java @@ -171,6 +171,7 @@ import static org.opensearch.indices.IndicesService.CLUSTER_REPLICATION_TYPE_SETTING; import static org.opensearch.indices.ShardLimitValidatorTests.createTestShardLimitService; import static org.opensearch.node.Node.NODE_ATTRIBUTES; +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.getRemoteStoreTranslogRepo; @@ -1657,7 +1658,12 @@ public void testNewIndexIsRemoteStoreBackedForRemoteStoreDirectionAndMixedMode() null ); - Map missingTranslogAttribute = Map.of(REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, "my-segment-repo-1"); + Map missingTranslogAttribute = Map.of( + REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY, + "cluster-state-repo-1", + REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, + "my-segment-repo-1" + ); DiscoveryNodes finalDiscoveryNodes = DiscoveryNodes.builder() .add(nonRemoteClusterManagerNode) @@ -2627,6 +2633,7 @@ private void verifyRemoteStoreIndexSettings( private DiscoveryNode getRemoteNode() { Map attributes = new HashMap<>(); + attributes.put(REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY, "my-cluster-rep-1"); attributes.put(REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, "my-segment-repo-1"); attributes.put(REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY, "my-translog-repo-1"); return new DiscoveryNode( diff --git a/server/src/test/java/org/opensearch/cluster/routing/allocation/FailedShardsRoutingTests.java b/server/src/test/java/org/opensearch/cluster/routing/allocation/FailedShardsRoutingTests.java index 5e3b74ee138ab..f8e1c609e6ee8 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/allocation/FailedShardsRoutingTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/allocation/FailedShardsRoutingTests.java @@ -68,6 +68,7 @@ import static org.opensearch.cluster.routing.ShardRoutingState.STARTED; import static org.opensearch.cluster.routing.ShardRoutingState.UNASSIGNED; import static org.opensearch.common.util.FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL; +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING; @@ -853,6 +854,8 @@ public void testPreferReplicaOnRemoteNodeForPrimaryPromotion() { // add a remote node and start primary shard Map remoteStoreNodeAttributes = Map.of( + REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY, + "REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY", REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, "REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_VALUE", REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY, diff --git a/server/src/test/java/org/opensearch/cluster/routing/allocation/RemoteStoreMigrationAllocationDeciderTests.java b/server/src/test/java/org/opensearch/cluster/routing/allocation/RemoteStoreMigrationAllocationDeciderTests.java index 5b29922f2400c..e6e81c94e7f32 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/allocation/RemoteStoreMigrationAllocationDeciderTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/allocation/RemoteStoreMigrationAllocationDeciderTests.java @@ -68,6 +68,7 @@ import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY; import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY; +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeService.Direction.NONE; import static org.opensearch.node.remotestore.RemoteStoreNodeService.Direction.REMOTE_STORE; import static org.opensearch.node.remotestore.RemoteStoreNodeService.MIGRATION_DIRECTION_SETTING; @@ -617,6 +618,7 @@ private DiscoveryNode getRemoteNode() { REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY, "REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_VALUE" ); + attributes.put(REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, "REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_VALUE"); return new DiscoveryNode( UUIDs.base64UUID(), buildNewFakeTransportAddress(), diff --git a/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java b/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java index a6db37285fe6f..2a34dd3580948 100644 --- a/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java +++ b/server/src/test/java/org/opensearch/index/remote/RemoteStoreUtilsTests.java @@ -67,6 +67,7 @@ import static org.opensearch.index.store.RemoteSegmentStoreDirectory.MetadataFilenameUtils.SEPARATOR; import static org.opensearch.index.translog.transfer.TranslogTransferMetadata.METADATA_SEPARATOR; import static org.opensearch.indices.RemoteStoreSettings.CLUSTER_REMOTE_STORE_PINNED_TIMESTAMP_ENABLED; +import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY; import static org.opensearch.node.remotestore.RemoteStoreNodeService.REMOTE_STORE_COMPATIBILITY_MODE_SETTING; @@ -540,6 +541,7 @@ private RoutingTable createRoutingTableAllShardsStarted( private Map getRemoteStoreNodeAttributes() { Map remoteStoreNodeAttributes = new HashMap<>(); + remoteStoreNodeAttributes.put(REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY, "my-cluster-repo-1"); remoteStoreNodeAttributes.put(REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, "my-segment-repo-1"); remoteStoreNodeAttributes.put(REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY, "my-translog-repo-1"); return remoteStoreNodeAttributes; diff --git a/test/framework/src/main/java/org/opensearch/index/shard/IndexShardTestUtils.java b/test/framework/src/main/java/org/opensearch/index/shard/IndexShardTestUtils.java index abf8f2a4da6c1..50a1751546293 100644 --- a/test/framework/src/main/java/org/opensearch/index/shard/IndexShardTestUtils.java +++ b/test/framework/src/main/java/org/opensearch/index/shard/IndexShardTestUtils.java @@ -21,6 +21,7 @@ import java.util.Map; public class IndexShardTestUtils { + public static final String MOCK_STATE_REPO_NAME = "state-test-repo"; public static final String MOCK_SEGMENT_REPO_NAME = "segment-test-repo"; public static final String MOCK_TLOG_REPO_NAME = "tlog-test-repo"; @@ -37,6 +38,7 @@ public static DiscoveryNode getFakeDiscoNode(String id) { public static DiscoveryNode getFakeRemoteEnabledNode(String id) { Map remoteNodeAttributes = new HashMap(); + remoteNodeAttributes.put(RemoteStoreNodeAttribute.REMOTE_STORE_CLUSTER_STATE_REPOSITORY_NAME_ATTRIBUTE_KEY, MOCK_STATE_REPO_NAME); remoteNodeAttributes.put(RemoteStoreNodeAttribute.REMOTE_STORE_SEGMENT_REPOSITORY_NAME_ATTRIBUTE_KEY, MOCK_SEGMENT_REPO_NAME); remoteNodeAttributes.put(RemoteStoreNodeAttribute.REMOTE_STORE_TRANSLOG_REPOSITORY_NAME_ATTRIBUTE_KEY, MOCK_TLOG_REPO_NAME); return new DiscoveryNode( From d2bc9fc3daaaa33273bace58c4a94d2ae3e7be5c Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Wed, 4 Sep 2024 17:18:09 +0530 Subject: [PATCH 6/8] Set Version to 2.17 for hashed prefix snapshots (#15656) Signed-off-by: Ashish Singh --- server/src/main/java/org/opensearch/repositories/IndexId.java | 4 ++-- .../main/java/org/opensearch/repositories/RepositoryData.java | 4 ++-- .../main/java/org/opensearch/snapshots/SnapshotsService.java | 2 +- .../test/java/org/opensearch/repositories/IndexIdTests.java | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/opensearch/repositories/IndexId.java b/server/src/main/java/org/opensearch/repositories/IndexId.java index 238dffbb46bde..5a9a757e31cb1 100644 --- a/server/src/main/java/org/opensearch/repositories/IndexId.java +++ b/server/src/main/java/org/opensearch/repositories/IndexId.java @@ -78,7 +78,7 @@ public IndexId(String name, String id, int shardPathType) { public IndexId(final StreamInput in) throws IOException { this.name = in.readString(); this.id = in.readString(); - if (in.getVersion().onOrAfter(Version.CURRENT)) { + if (in.getVersion().onOrAfter(Version.V_2_17_0)) { this.shardPathType = in.readVInt(); } else { this.shardPathType = DEFAULT_SHARD_PATH_TYPE; @@ -145,7 +145,7 @@ private int computeHashCode() { public void writeTo(final StreamOutput out) throws IOException { out.writeString(name); out.writeString(id); - if (out.getVersion().onOrAfter(Version.CURRENT)) { + if (out.getVersion().onOrAfter(Version.V_2_17_0)) { out.writeVInt(shardPathType); } } diff --git a/server/src/main/java/org/opensearch/repositories/RepositoryData.java b/server/src/main/java/org/opensearch/repositories/RepositoryData.java index 7921a9589b443..f52f1fa76f6bf 100644 --- a/server/src/main/java/org/opensearch/repositories/RepositoryData.java +++ b/server/src/main/java/org/opensearch/repositories/RepositoryData.java @@ -551,7 +551,7 @@ public List resolveNewIndices(List indicesToResolve, Map indexIds = repositoryData.resolveNewIndices( diff --git a/server/src/test/java/org/opensearch/repositories/IndexIdTests.java b/server/src/test/java/org/opensearch/repositories/IndexIdTests.java index 3b719d287aa9b..bcd75f9a47ad8 100644 --- a/server/src/test/java/org/opensearch/repositories/IndexIdTests.java +++ b/server/src/test/java/org/opensearch/repositories/IndexIdTests.java @@ -86,7 +86,7 @@ public void testEqualsAndHashCode() { public void testSerialization() throws IOException { IndexId indexId = new IndexId(randomAlphaOfLength(8), UUIDs.randomBase64UUID(), randomIntBetween(0, 2)); BytesStreamOutput out = new BytesStreamOutput(); - out.setVersion(Version.CURRENT); + out.setVersion(Version.V_2_17_0); indexId.writeTo(out); assertEquals(indexId, new IndexId(out.bytes().streamInput())); } From ad1df9e7b765d2e777cf77dec129a6cd8f11af8a Mon Sep 17 00:00:00 2001 From: "Samuel.G" <1148690954@qq.com> Date: Wed, 4 Sep 2024 20:57:09 +0800 Subject: [PATCH 7/8] fix wildcard term & terms query & add YAML REST test (#15607) * fix wildcard term & terms query & add YAML REST test Signed-off-by: gesong.samuel * add CHANGELOG Signed-off-by: gesong.samuel --------- Signed-off-by: gesong.samuel Co-authored-by: gesong.samuel --- CHANGELOG.md | 1 + .../test/search/270_wildcard_fieldtype_queries.yml | 10 ++++++++++ .../opensearch/index/mapper/WildcardFieldMapper.java | 4 ++-- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f67e9a4c257f7..71868de93812e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -106,6 +106,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix split response processor not included in allowlist ([#15393](https://github.com/opensearch-project/OpenSearch/pull/15393)) - Fix unchecked cast in dynamic action map getter ([#15394](https://github.com/opensearch-project/OpenSearch/pull/15394)) - Fix null values indexed as "null" strings in flat_object field ([#14069](https://github.com/opensearch-project/OpenSearch/pull/14069)) +- Fix terms query on wildcard field returns nothing ([#15607](https://github.com/opensearch-project/OpenSearch/pull/15607)) ### Security diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/270_wildcard_fieldtype_queries.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/270_wildcard_fieldtype_queries.yml index 05b6b2e5ed712..c449a06cf633d 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/270_wildcard_fieldtype_queries.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/270_wildcard_fieldtype_queries.yml @@ -227,3 +227,13 @@ setup: my_field: value: ".*" - match: { hits.total.value: 5 } +--- +"terms query on wildcard field matches": + - do: + search: + index: test + body: + query: + terms: { my_field: ["AbCd"] } + - match: { hits.total.value: 1 } + - match: { hits.hits.0._id: "5" } diff --git a/server/src/main/java/org/opensearch/index/mapper/WildcardFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/WildcardFieldMapper.java index 4998a822917b4..cd95e320209ee 100644 --- a/server/src/main/java/org/opensearch/index/mapper/WildcardFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/WildcardFieldMapper.java @@ -635,7 +635,7 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower @Override public Query termQueryCaseInsensitive(Object value, QueryShardContext context) { - return wildcardQuery(value.toString(), MultiTermQuery.CONSTANT_SCORE_REWRITE, true, context); + return wildcardQuery(BytesRefs.toString(value), MultiTermQuery.CONSTANT_SCORE_REWRITE, true, context); } @Override @@ -649,7 +649,7 @@ public Query termsQuery(List values, QueryShardContext context) { Set expectedValues = new HashSet<>(); StringBuilder pattern = new StringBuilder(); for (Object value : values) { - String stringVal = value.toString(); + String stringVal = BytesRefs.toString(value); builder.add(matchAllTermsQuery(name(), getRequiredNGrams(stringVal)), BooleanClause.Occur.SHOULD); expectedValues.add(stringVal); if (pattern.length() > 0) { From 4f57d6aab21594a5ac20d26abbbba959aba1ab75 Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Wed, 4 Sep 2024 05:58:28 -0700 Subject: [PATCH 8/8] Fix approximate pointrangequery tests (#15590) Signed-off-by: Harsha Vamsi Kalluri --- .../ApproximatePointRangeQuery.java | 15 ++++++++++----- .../ApproximatePointRangeQueryTests.java | 19 +++++++++++-------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java b/server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java index cee8bc43d7ffd..8076da6ab970b 100644 --- a/server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java +++ b/server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java @@ -146,10 +146,11 @@ public void grow(int count) { public void visit(int docID) { // it is possible that size < 1024 and docCount < size but we will continue to count through all the 1024 docs // and collect less, but it won't hurt performance - if (docCount[0] < size) { - adder.add(docID); - docCount[0]++; + if (docCount[0] >= size) { + return; } + adder.add(docID); + docCount[0]++; } @Override @@ -231,7 +232,7 @@ private void intersectRight(PointValues.PointTree pointTree, PointValues.Interse public void intersectLeft(PointValues.IntersectVisitor visitor, PointValues.PointTree pointTree, long[] docCount) throws IOException { PointValues.Relation r = visitor.compare(pointTree.getMinPackedValue(), pointTree.getMaxPackedValue()); - if (docCount[0] > size) { + if (docCount[0] >= size) { return; } switch (r) { @@ -279,7 +280,7 @@ public void intersectLeft(PointValues.IntersectVisitor visitor, PointValues.Poin public void intersectRight(PointValues.IntersectVisitor visitor, PointValues.PointTree pointTree, long[] docCount) throws IOException { PointValues.Relation r = visitor.compare(pointTree.getMinPackedValue(), pointTree.getMaxPackedValue()); - if (docCount[0] > size) { + if (docCount[0] >= size) { return; } switch (r) { @@ -435,6 +436,10 @@ public boolean canApproximate(SearchContext context) { if (!(context.query() instanceof ApproximateIndexOrDocValuesQuery)) { return false; } + // size 0 could be set for caching + if (context.from() + context.size() == 0) { + this.setSize(10_000); + } this.setSize(Math.max(context.from() + context.size(), context.trackTotalHitsUpTo())); if (context.request() != null && context.request().source() != null) { FieldSortBuilder primarySortField = FieldSortBuilder.getPrimaryFieldSortOrNull(context.request().source()); diff --git a/server/src/test/java/org/opensearch/search/approximate/ApproximatePointRangeQueryTests.java b/server/src/test/java/org/opensearch/search/approximate/ApproximatePointRangeQueryTests.java index 7fb75e43da6ec..1eaea6a9e9d47 100644 --- a/server/src/test/java/org/opensearch/search/approximate/ApproximatePointRangeQueryTests.java +++ b/server/src/test/java/org/opensearch/search/approximate/ApproximatePointRangeQueryTests.java @@ -127,9 +127,9 @@ public void testApproximateRangeWithSizeUnderDefault() throws IOException { } doc.add(new LongPoint("point", scratch)); iw.addDocument(doc); - if (i % 15 == 0) iw.flush(); } iw.flush(); + iw.forceMerge(1); try (IndexReader reader = iw.getReader()) { try { long lower = 0; @@ -168,6 +168,7 @@ public void testApproximateRangeWithSizeOverDefault() throws IOException { iw.addDocument(doc); } iw.flush(); + iw.forceMerge(1); try (IndexReader reader = iw.getReader()) { try { long lower = 0; @@ -185,7 +186,7 @@ protected String toString(int dimension, byte[] value) { }; IndexSearcher searcher = new IndexSearcher(reader); TopDocs topDocs = searcher.search(approximateQuery, 11000); - assertEquals(topDocs.totalHits, new TotalHits(11001, TotalHits.Relation.GREATER_THAN_OR_EQUAL_TO)); + assertEquals(topDocs.totalHits, new TotalHits(11000, TotalHits.Relation.EQUAL_TO)); } catch (IOException e) { throw new RuntimeException(e); } @@ -213,6 +214,7 @@ public void testApproximateRangeShortCircuit() throws IOException { if (i % 10 == 0) iw.flush(); } iw.flush(); + iw.forceMerge(1); try (IndexReader reader = iw.getReader()) { try { long lower = 0; @@ -258,6 +260,7 @@ public void testApproximateRangeShortCircuitAscSort() throws IOException { iw.addDocument(doc); } iw.flush(); + iw.forceMerge(1); try (IndexReader reader = iw.getReader()) { try { long lower = 0; @@ -284,12 +287,12 @@ protected String toString(int dimension, byte[] value) { assertNotEquals(topDocs.totalHits, topDocs1.totalHits); assertEquals(topDocs.totalHits, new TotalHits(10, TotalHits.Relation.EQUAL_TO)); assertEquals(topDocs1.totalHits, new TotalHits(21, TotalHits.Relation.EQUAL_TO)); - assertEquals(topDocs.scoreDocs[0].doc, 0); - assertEquals(topDocs.scoreDocs[1].doc, 1); - assertEquals(topDocs.scoreDocs[2].doc, 2); - assertEquals(topDocs.scoreDocs[3].doc, 3); - assertEquals(topDocs.scoreDocs[4].doc, 4); - assertEquals(topDocs.scoreDocs[5].doc, 5); + assertEquals(topDocs.scoreDocs[0].doc, topDocs1.scoreDocs[0].doc); + assertEquals(topDocs.scoreDocs[1].doc, topDocs1.scoreDocs[1].doc); + assertEquals(topDocs.scoreDocs[2].doc, topDocs1.scoreDocs[2].doc); + assertEquals(topDocs.scoreDocs[3].doc, topDocs1.scoreDocs[3].doc); + assertEquals(topDocs.scoreDocs[4].doc, topDocs1.scoreDocs[4].doc); + assertEquals(topDocs.scoreDocs[5].doc, topDocs1.scoreDocs[5].doc); } catch (IOException e) { throw new RuntimeException(e);