Skip to content

Commit 736551d

Browse files
ansjcyansjcy
authored
sync bug fixes from core to the plugin repo (opensearch-project#13)
Signed-off-by: Chenyang Ji <cyji@amazon.com>
1 parent 0472a96 commit 736551d

File tree

8 files changed

+161
-19
lines changed

8 files changed

+161
-19
lines changed

‎.editorconfig

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# EditorConfig: http://editorconfig.org/
2+
3+
root = true
4+
5+
[*]
6+
charset = utf-8
7+
trim_trailing_whitespace = true
8+
insert_final_newline = true
9+
indent_style = space
10+
11+
[*.gradle]
12+
indent_size = 2
13+
14+
[*.groovy]
15+
indent_size = 4
16+
17+
[*.java]
18+
indent_size = 4
19+
20+
[*.json]
21+
indent_size = 2
22+
23+
[*.py]
24+
indent_size = 2
25+
26+
[*.sh]
27+
indent_size = 2
28+
29+
[*.{yml,yaml}]
30+
indent_size = 2
31+
32+
[*.{xsd,xml}]
33+
indent_size = 4

‎src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public final class QueryInsightsListener extends SearchRequestOperationsListener
5252
/**
5353
* Constructor for QueryInsightsListener
5454
*
55-
* @param clusterService The Node's cluster service.
55+
* @param clusterService The Node's cluster service.
5656
* @param queryInsightsService The topQueriesByLatencyService associated with this listener
5757
*/
5858
@Inject
@@ -91,7 +91,7 @@ public QueryInsightsListener(final ClusterService clusterService, final QueryIns
9191
* and query insights services.
9292
*
9393
* @param metricType {@link MetricType}
94-
* @param enabled boolean
94+
* @param enabled boolean
9595
*/
9696
public void setEnableTopQueries(final MetricType metricType, final boolean enabled) {
9797
boolean isAllMetricsDisabled = !queryInsightsService.isEnabled();

‎src/main/java/org/opensearch/plugin/insights/core/service/TopQueriesService.java

+5-6
Original file line numberDiff line numberDiff line change
@@ -136,24 +136,23 @@ public int getTopNSize() {
136136
* @param size the wanted top N size
137137
*/
138138
public void validateTopNSize(final int size) {
139-
if (size > QueryInsightsSettings.MAX_N_SIZE) {
139+
if (size < 1 || size > QueryInsightsSettings.MAX_N_SIZE) {
140140
throw new IllegalArgumentException(
141141
"Top N size setting for ["
142142
+ metricType
143143
+ "]"
144-
+ " should be smaller than max top N size ["
144+
+ " should be between 1 and "
145145
+ QueryInsightsSettings.MAX_N_SIZE
146-
+ "was ("
146+
+ ", was ("
147147
+ size
148-
+ " > "
149-
+ QueryInsightsSettings.MAX_N_SIZE
150148
+ ")"
151149
);
152150
}
153151
}
154152

155153
/**
156154
* Set enable flag for the service
155+
*
157156
* @param enabled boolean
158157
*/
159158
public void setEnabled(final boolean enabled) {
@@ -251,7 +250,7 @@ public void validateExporterConfig(Settings settings) {
251250
/**
252251
* Get all top queries records that are in the current top n queries store
253252
* Optionally include top N records from the last window.
254-
*
253+
* <p>
255254
* By default, return the records in sorted order.
256255
*
257256
* @param includeLastWindow if the top N queries from the last window should be included

‎src/main/java/org/opensearch/plugin/insights/rules/model/Attribute.java

+72-1
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,18 @@
88

99
package org.opensearch.plugin.insights.rules.model;
1010

11+
import org.apache.lucene.util.ArrayUtil;
1112
import org.opensearch.core.common.io.stream.StreamInput;
1213
import org.opensearch.core.common.io.stream.StreamOutput;
14+
import org.opensearch.core.common.io.stream.Writeable;
15+
import org.opensearch.core.tasks.resourcetracker.TaskResourceInfo;
1316

1417
import java.io.IOException;
18+
import java.util.Collections;
19+
import java.util.HashMap;
20+
import java.util.List;
1521
import java.util.Locale;
22+
import java.util.Map;
1623

1724
/**
1825
* Valid attributes for a search query record
@@ -65,14 +72,78 @@ static Attribute readFromStream(final StreamInput in) throws IOException {
6572
/**
6673
* Write Attribute to a StreamOutput
6774
*
68-
* @param out the StreamOutput to write
75+
* @param out the StreamOutput to write
6976
* @param attribute the Attribute to write
7077
* @throws IOException IOException
7178
*/
7279
static void writeTo(final StreamOutput out, final Attribute attribute) throws IOException {
7380
out.writeString(attribute.toString());
7481
}
7582

83+
/**
84+
* Write Attribute value to a StreamOutput
85+
*
86+
* @param out the StreamOutput to write
87+
* @param attributeValue the Attribute value to write
88+
*/
89+
@SuppressWarnings("unchecked")
90+
public static void writeValueTo(StreamOutput out, Object attributeValue) throws IOException {
91+
if (attributeValue instanceof List) {
92+
out.writeList((List<? extends Writeable>) attributeValue);
93+
} else {
94+
out.writeGenericValue(attributeValue);
95+
}
96+
}
97+
98+
/**
99+
* Read attribute value from the input stream given the Attribute type
100+
*
101+
* @param in the {@link StreamInput} input to read
102+
* @param attribute attribute type to differentiate between Source and others
103+
* @return parse value
104+
* @throws IOException IOException
105+
*/
106+
public static Object readAttributeValue(StreamInput in, Attribute attribute) throws IOException {
107+
if (attribute == Attribute.TASK_RESOURCE_USAGES) {
108+
return in.readList(TaskResourceInfo::readFromStream);
109+
} else {
110+
return in.readGenericValue();
111+
}
112+
}
113+
114+
/**
115+
* Read attribute map from the input stream
116+
*
117+
* @param in the {@link StreamInput} to read
118+
* @return parsed attribute map
119+
* @throws IOException IOException
120+
*/
121+
public static Map<Attribute, Object> readAttributeMap(StreamInput in) throws IOException {
122+
int size = readArraySize(in);
123+
if (size == 0) {
124+
return Collections.emptyMap();
125+
}
126+
Map<Attribute, Object> map = new HashMap<>(size);
127+
128+
for (int i = 0; i < size; i++) {
129+
Attribute key = readFromStream(in);
130+
Object value = readAttributeValue(in, key);
131+
map.put(key, value);
132+
}
133+
return map;
134+
}
135+
136+
private static int readArraySize(StreamInput in) throws IOException {
137+
final int arraySize = in.readVInt();
138+
if (arraySize > ArrayUtil.MAX_ARRAY_LENGTH) {
139+
throw new IllegalStateException("array length must be <= to " + ArrayUtil.MAX_ARRAY_LENGTH + " but was: " + arraySize);
140+
}
141+
if (arraySize < 0) {
142+
throw new NegativeArraySizeException("array size must be positive but was: " + arraySize);
143+
}
144+
return arraySize;
145+
}
146+
76147
@Override
77148
public String toString() {
78149
return this.name().toLowerCase(Locale.ROOT);

‎src/main/java/org/opensearch/plugin/insights/rules/model/SearchQueryRecord.java

+6-2
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public SearchQueryRecord(final StreamInput in) throws IOException, ClassCastExce
4343
measurements = new HashMap<>();
4444
in.readMap(MetricType::readFromStream, StreamInput::readGenericValue)
4545
.forEach(((metricType, o) -> measurements.put(metricType, metricType.parseValue(o))));
46-
this.attributes = in.readMap(Attribute::readFromStream, StreamInput::readGenericValue);
46+
this.attributes = Attribute.readAttributeMap(in);
4747
}
4848

4949
/**
@@ -132,7 +132,11 @@ public XContentBuilder toXContent(final XContentBuilder builder, final ToXConten
132132
public void writeTo(final StreamOutput out) throws IOException {
133133
out.writeLong(timestamp);
134134
out.writeMap(measurements, (stream, metricType) -> MetricType.writeTo(out, metricType), StreamOutput::writeGenericValue);
135-
out.writeMap(attributes, (stream, attribute) -> Attribute.writeTo(out, attribute), StreamOutput::writeGenericValue);
135+
out.writeMap(
136+
attributes,
137+
(stream, attribute) -> Attribute.writeTo(out, attribute),
138+
(stream, attributeValue) -> Attribute.writeValueTo(out, attributeValue)
139+
);
136140
}
137141

138142
/**

‎src/test/java/org/opensearch/plugin/insights/QueryInsightsTestUtils.java

+23-2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
import org.opensearch.cluster.node.DiscoveryNode;
1313
import org.opensearch.common.settings.ClusterSettings;
1414
import org.opensearch.common.util.Maps;
15+
import org.opensearch.core.tasks.resourcetracker.TaskResourceInfo;
16+
import org.opensearch.core.tasks.resourcetracker.TaskResourceUsage;
1517
import org.opensearch.core.xcontent.ToXContent;
1618
import org.opensearch.core.xcontent.XContentBuilder;
1719
import org.opensearch.plugin.insights.rules.action.top_queries.TopQueries;
@@ -80,6 +82,25 @@ public static List<SearchQueryRecord> generateQueryInsightRecords(int lower, int
8082
attributes.put(Attribute.TOTAL_SHARDS, randomIntBetween(1, 100));
8183
attributes.put(Attribute.INDICES, randomArray(1, 3, Object[]::new, () -> randomAlphaOfLengthBetween(5, 10)));
8284
attributes.put(Attribute.PHASE_LATENCY_MAP, phaseLatencyMap);
85+
attributes.put(
86+
Attribute.TASK_RESOURCE_USAGES,
87+
List.of(
88+
new TaskResourceInfo(
89+
randomAlphaOfLengthBetween(5, 10),
90+
randomLongBetween(1, 1000),
91+
randomLongBetween(1, 1000),
92+
randomAlphaOfLengthBetween(5, 10),
93+
new TaskResourceUsage(randomLongBetween(1, 1000), randomLongBetween(1, 1000))
94+
),
95+
new TaskResourceInfo(
96+
randomAlphaOfLengthBetween(5, 10),
97+
randomLongBetween(1, 1000),
98+
randomLongBetween(1, 1000),
99+
randomAlphaOfLengthBetween(5, 10),
100+
new TaskResourceUsage(randomLongBetween(1, 1000), randomLongBetween(1, 1000))
101+
)
102+
)
103+
);
83104

84105
records.add(new SearchQueryRecord(timestamp, measurements, attributes));
85106
timestamp += interval;
@@ -163,8 +184,8 @@ public static boolean checkRecordsEquals(List<SearchQueryRecord> records1, List<
163184
return false;
164185
} else if (value instanceof Map
165186
&& !Maps.deepEquals((Map<Object, Object>) value, (Map<Object, Object>) attributes2.get(attribute))) {
166-
return false;
167-
}
187+
return false;
188+
}
168189
}
169190
}
170191
return true;

‎src/test/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListenerTests.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ public void testOnRequestEnd() throws InterruptedException {
108108
Collections.singletonMap(Task.X_OPAQUE_ID, "userLabel")
109109
);
110110

111-
String[] indices = new String[] { "index-1", "index-2" };
111+
String[] indices = new String[]{"index-1", "index-2"};
112112

113113
Map<String, Long> phaseLatencyMap = new HashMap<>();
114114
phaseLatencyMap.put("expand", 0L);
@@ -157,7 +157,7 @@ public void testConcurrentOnRequestEnd() throws InterruptedException {
157157
Collections.singletonMap(Task.X_OPAQUE_ID, "userLabel")
158158
);
159159

160-
String[] indices = new String[] { "index-1", "index-2" };
160+
String[] indices = new String[]{"index-1", "index-2"};
161161

162162
Map<String, Long> phaseLatencyMap = new HashMap<>();
163163
phaseLatencyMap.put("expand", 0L);

‎src/test/java/org/opensearch/plugin/insights/core/service/TopQueriesServiceTests.java

+18-4
Original file line numberDiff line numberDiff line change
@@ -75,12 +75,22 @@ public void testSmallNSize() {
7575
}
7676

7777
public void testValidateTopNSize() {
78-
assertThrows(IllegalArgumentException.class, () -> { topQueriesService.validateTopNSize(QueryInsightsSettings.MAX_N_SIZE + 1); });
78+
assertThrows(IllegalArgumentException.class, () -> {
79+
topQueriesService.validateTopNSize(QueryInsightsSettings.MAX_N_SIZE + 1);
80+
});
81+
}
82+
83+
public void testValidateNegativeTopNSize() {
84+
assertThrows(IllegalArgumentException.class, () -> {
85+
topQueriesService.validateTopNSize(-1);
86+
});
7987
}
8088

8189
public void testGetTopQueriesWhenNotEnabled() {
8290
topQueriesService.setEnabled(false);
83-
assertThrows(IllegalArgumentException.class, () -> { topQueriesService.getTopQueriesRecords(false); });
91+
assertThrows(IllegalArgumentException.class, () -> {
92+
topQueriesService.getTopQueriesRecords(false);
93+
});
8494
}
8595

8696
public void testValidateWindowSize() {
@@ -90,8 +100,12 @@ public void testValidateWindowSize() {
90100
assertThrows(IllegalArgumentException.class, () -> {
91101
topQueriesService.validateWindowSize(new TimeValue(QueryInsightsSettings.MIN_WINDOW_SIZE.getSeconds() - 1, TimeUnit.SECONDS));
92102
});
93-
assertThrows(IllegalArgumentException.class, () -> { topQueriesService.validateWindowSize(new TimeValue(2, TimeUnit.DAYS)); });
94-
assertThrows(IllegalArgumentException.class, () -> { topQueriesService.validateWindowSize(new TimeValue(7, TimeUnit.MINUTES)); });
103+
assertThrows(IllegalArgumentException.class, () -> {
104+
topQueriesService.validateWindowSize(new TimeValue(2, TimeUnit.DAYS));
105+
});
106+
assertThrows(IllegalArgumentException.class, () -> {
107+
topQueriesService.validateWindowSize(new TimeValue(7, TimeUnit.MINUTES));
108+
});
95109
}
96110

97111
private static void runUntilTimeoutOrFinish(DeterministicTaskQueue deterministicTaskQueue, long duration) {

0 commit comments

Comments
 (0)