Skip to content

Commit e4caace

Browse files
peteralfonsipeteralfonsi
authored
Merge branch 'main' into merging-percentiles
Signed-off-by: Peter Alfonsi <[email protected]>
2 parents 3cea57c + b82ba83 commit e4caace

File tree

13 files changed

+500
-205
lines changed

13 files changed

+500
-205
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
3131
- Upgrade crypto kms plugin dependencies for AWS SDK v2.x. ([#18268](https://github.com/opensearch-project/OpenSearch/pull/18268))
3232
- Add support for `matched_fields` with the unified highlighter ([#18164](https://github.com/opensearch-project/OpenSearch/issues/18164))
3333
- [repository-s3] Add support for SSE-KMS and S3 bucket owner verification ([#18312](https://github.com/opensearch-project/OpenSearch/pull/18312))
34+
- Optimize gRPC perf by passing by reference ([#18303](https://github.com/opensearch-project/OpenSearch/pull/18303))
3435
- Added File Cache Stats - Involves Block level as well as full file level stats ([#17538](https://github.com/opensearch-project/OpenSearch/issues/17479))
3536

3637
### Changed

‎CODE_OF_CONDUCT.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,4 @@ This code of conduct applies to all spaces provided by the OpenSource project in
2222
* Advocating for or encouraging any of the above behaviors.
2323
* Enforcement and Reporting Code of Conduct Issues:
2424

25-
Instances of abusive, harassing, or otherwise unacceptable behavior may be reported. [Contact us](mailto:[email protected]). All complaints will be reviewed and investigated and will result in a response that is deemed necessary and appropriate to the circumstances.
25+
Instances of abusive, harassing, or otherwise unacceptable behavior may be reported. [Contact us](mailto:[email protected]). All complaints will be reviewed and investigated and will result in a response that is deemed necessary and appropriate to the circumstances.

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343

4444
## Code of Conduct
4545

46-
This project has adopted the [Amazon Open Source Code of Conduct](CODE_OF_CONDUCT.md). For more information see the [Code of Conduct FAQ](https://aws.github.io/code-of-conduct-faq), or contact [[email protected]](mailto:[email protected]) with any additional questions or comments.
46+
The project's [Code of Conduct](CODE_OF_CONDUCT.md) outlines our expectations for all participants in our community, based on the [OpenSearch Code of Conduct](https://opensearch.org/code-of-conduct/). Please contact [[email protected]](mailto:[email protected]) with any additional questions or comments.
4747

4848
## Security
4949
If you discover a potential security issue in this project we ask that you notify OpenSearch Security directly via email to [email protected]. Please do **not** create a public GitHub issue.

‎plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/response/common/FieldValueProtoUtils.java‎

Lines changed: 46 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -35,42 +35,56 @@ private FieldValueProtoUtils() {
3535
*/
3636
public static FieldValue toProto(Object javaObject) {
3737
FieldValue.Builder fieldValueBuilder = FieldValue.newBuilder();
38+
toProto(javaObject, fieldValueBuilder);
39+
return fieldValueBuilder.build();
40+
}
3841

39-
if (javaObject instanceof Integer) {
40-
// Integer
41-
fieldValueBuilder.setGeneralNumber(GeneralNumber.newBuilder().setInt32Value((int) javaObject).build());
42-
} else if (javaObject instanceof Long) {
43-
// Long
44-
fieldValueBuilder.setGeneralNumber(GeneralNumber.newBuilder().setInt64Value((long) javaObject).build());
45-
} else if (javaObject instanceof Double) {
46-
// Double
47-
fieldValueBuilder.setGeneralNumber(GeneralNumber.newBuilder().setDoubleValue((double) javaObject).build());
48-
} else if (javaObject instanceof Float) {
49-
// Float
50-
fieldValueBuilder.setGeneralNumber(GeneralNumber.newBuilder().setFloatValue((float) javaObject).build());
51-
} else if (javaObject instanceof String) {
52-
// String
53-
fieldValueBuilder.setStringValue((String) javaObject);
54-
} else if (javaObject instanceof Boolean) {
55-
// Boolean
56-
fieldValueBuilder.setBoolValue((Boolean) javaObject);
57-
} else if (javaObject instanceof Enum) {
58-
// Enum
59-
fieldValueBuilder.setStringValue(javaObject.toString());
60-
} else if (javaObject instanceof Map) {
61-
// Map
62-
ObjectMap.Builder objectMapBuilder = ObjectMap.newBuilder();
42+
/**
43+
* Converts a generic Java Object to its Protocol Buffer FieldValue representation.
44+
* It handles various Java types (Integer, Long, Double, Float, String, Boolean, Enum, Map)
45+
* and converts them to the appropriate FieldValue type.
46+
*
47+
* @param javaObject The Java object to convert
48+
* @param fieldValueBuilder The builder to populate with the Java object data
49+
* @throws IllegalArgumentException if the Java object type cannot be converted
50+
*/
51+
public static void toProto(Object javaObject, FieldValue.Builder fieldValueBuilder) {
52+
if (javaObject == null) {
53+
throw new IllegalArgumentException("Cannot convert null to FieldValue");
54+
}
6355

64-
@SuppressWarnings("unchecked")
65-
Map<String, Object> fieldMap = (Map<String, Object>) javaObject;
66-
for (Map.Entry<String, Object> entry : fieldMap.entrySet()) {
67-
objectMapBuilder.putFields(entry.getKey(), ObjectMapProtoUtils.toProto(entry.getValue()));
56+
switch (javaObject) {
57+
case String s -> fieldValueBuilder.setStringValue(s);
58+
case Integer i -> fieldValueBuilder.setGeneralNumber(GeneralNumber.newBuilder().setInt32Value(i).build());
59+
case Long l -> fieldValueBuilder.setGeneralNumber(GeneralNumber.newBuilder().setInt64Value(l).build());
60+
case Double d -> fieldValueBuilder.setGeneralNumber(GeneralNumber.newBuilder().setDoubleValue(d).build());
61+
case Float f -> fieldValueBuilder.setGeneralNumber(GeneralNumber.newBuilder().setFloatValue(f).build());
62+
case Boolean b -> fieldValueBuilder.setBoolValue(b);
63+
case Enum<?> e -> fieldValueBuilder.setStringValue(e.toString());
64+
case Map<?, ?> m -> {
65+
@SuppressWarnings("unchecked")
66+
Map<String, Object> map = (Map<String, Object>) m;
67+
handleMapValue(map, fieldValueBuilder);
6868
}
69-
fieldValueBuilder.setObjectMap(objectMapBuilder.build());
70-
} else {
71-
throw new IllegalArgumentException("Cannot convert " + javaObject.toString() + " to google.protobuf.Struct");
69+
default -> throw new IllegalArgumentException("Cannot convert " + javaObject + " to FieldValue");
7270
}
71+
}
7372

74-
return fieldValueBuilder.build();
73+
/**
74+
* Helper method to handle Map values.
75+
*
76+
* @param map The map to convert
77+
* @param fieldValueBuilder The builder to populate with the map data
78+
*/
79+
@SuppressWarnings("unchecked")
80+
private static void handleMapValue(Map<String, Object> map, FieldValue.Builder fieldValueBuilder) {
81+
ObjectMap.Builder objectMapBuilder = ObjectMap.newBuilder();
82+
83+
// Process each map entry
84+
for (Map.Entry<String, Object> entry : map.entrySet()) {
85+
objectMapBuilder.putFields(entry.getKey(), ObjectMapProtoUtils.toProto(entry.getValue()));
86+
}
87+
88+
fieldValueBuilder.setObjectMap(objectMapBuilder.build());
7589
}
7690
}

‎plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/response/common/ObjectMapProtoUtils.java‎

Lines changed: 66 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -26,58 +26,83 @@ private ObjectMapProtoUtils() {
2626
* Converts a generic Java Object to its Protocol Buffer representation.
2727
*
2828
* @param javaObject The java object to convert
29-
* @return A Protobuf builder .google.protobuf.Struct representation
29+
* @return A Protobuf ObjectMap.Value representation
3030
*/
3131
public static ObjectMap.Value toProto(Object javaObject) {
3232
ObjectMap.Value.Builder valueBuilder = ObjectMap.Value.newBuilder();
33+
toProto(javaObject, valueBuilder);
34+
return valueBuilder.build();
35+
}
3336

37+
/**
38+
* Converts a generic Java Object to its Protocol Buffer representation.
39+
*
40+
* @param javaObject The java object to convert
41+
* @param valueBuilder The builder to populate with the java object data
42+
*/
43+
public static void toProto(Object javaObject, ObjectMap.Value.Builder valueBuilder) {
3444
if (javaObject == null) {
3545
// Null
3646
valueBuilder.setNullValue(NullValue.NULL_VALUE_NULL);
47+
return;
3748
}
38-
// TODO does the order we set int, long, double, float, matter?
39-
else if (javaObject instanceof Integer) {
40-
// Integer
41-
valueBuilder.setInt32((int) javaObject);
42-
} else if (javaObject instanceof Long) {
43-
// Long
44-
valueBuilder.setInt64((long) javaObject);
45-
} else if (javaObject instanceof Double) {
46-
// Double
47-
valueBuilder.setDouble((double) javaObject);
48-
} else if (javaObject instanceof Float) {
49-
// Float
50-
valueBuilder.setFloat((float) javaObject);
51-
} else if (javaObject instanceof String) {
52-
// String
53-
valueBuilder.setString((String) javaObject);
54-
} else if (javaObject instanceof Boolean) {
55-
// Boolean
56-
valueBuilder.setBool((Boolean) javaObject);
57-
} else if (javaObject instanceof Enum) {
58-
// Enum
59-
valueBuilder.setString(javaObject.toString());
60-
} else if (javaObject instanceof List) {
61-
// List
62-
ObjectMap.ListValue.Builder listBuilder = ObjectMap.ListValue.newBuilder();
63-
for (Object listEntry : (List) javaObject) {
64-
listBuilder.addValue(toProto(listEntry));
65-
}
66-
valueBuilder.setListValue(listBuilder.build());
67-
} else if (javaObject instanceof Map) {
68-
// Map
69-
ObjectMap.Builder objectMapBuilder = ObjectMap.newBuilder();
7049

71-
@SuppressWarnings("unchecked")
72-
Map<String, Object> fieldMap = (Map<String, Object>) javaObject;
73-
for (Map.Entry<String, Object> entry : fieldMap.entrySet()) {
74-
objectMapBuilder.putFields(entry.getKey(), toProto(entry.getValue()));
50+
switch (javaObject) {
51+
case String s -> valueBuilder.setString(s);
52+
case Integer i -> valueBuilder.setInt32(i);
53+
case Long l -> valueBuilder.setInt64(l);
54+
case Double d -> valueBuilder.setDouble(d);
55+
case Float f -> valueBuilder.setFloat(f);
56+
case Boolean b -> valueBuilder.setBool(b);
57+
case Enum<?> e -> valueBuilder.setString(e.toString());
58+
case List<?> list -> handleListValue(list, valueBuilder);
59+
case Map<?, ?> m -> {
60+
@SuppressWarnings("unchecked")
61+
Map<String, Object> map = (Map<String, Object>) m;
62+
handleMapValue(map, valueBuilder);
7563
}
76-
valueBuilder.setObjectMap(objectMapBuilder.build());
77-
} else {
78-
throw new IllegalArgumentException("Cannot convert " + javaObject.toString() + " to google.protobuf.Struct");
64+
default -> throw new IllegalArgumentException("Cannot convert " + javaObject + " to google.protobuf.Struct");
7965
}
66+
}
8067

81-
return valueBuilder.build();
68+
/**
69+
* Helper method to handle List values.
70+
*
71+
* @param list The list to convert
72+
* @param valueBuilder The builder to populate with the list data
73+
*/
74+
private static void handleListValue(List<?> list, ObjectMap.Value.Builder valueBuilder) {
75+
ObjectMap.ListValue.Builder listBuilder = ObjectMap.ListValue.newBuilder();
76+
77+
// Process each list entry
78+
for (Object listEntry : list) {
79+
// Create a new builder for each list entry
80+
ObjectMap.Value.Builder entryBuilder = ObjectMap.Value.newBuilder();
81+
toProto(listEntry, entryBuilder);
82+
listBuilder.addValue(entryBuilder.build());
83+
}
84+
85+
valueBuilder.setListValue(listBuilder.build());
86+
}
87+
88+
/**
89+
* Helper method to handle Map values.
90+
*
91+
* @param map The map to convert
92+
* @param valueBuilder The builder to populate with the map data
93+
*/
94+
@SuppressWarnings("unchecked")
95+
private static void handleMapValue(Map<String, Object> map, ObjectMap.Value.Builder valueBuilder) {
96+
ObjectMap.Builder objectMapBuilder = ObjectMap.newBuilder();
97+
98+
// Process each map entry
99+
for (Map.Entry<String, Object> entry : map.entrySet()) {
100+
// Create a new builder for each map value
101+
ObjectMap.Value.Builder entryValueBuilder = ObjectMap.Value.newBuilder();
102+
toProto(entry.getValue(), entryValueBuilder);
103+
objectMapBuilder.putFields(entry.getKey(), entryValueBuilder.build());
104+
}
105+
106+
valueBuilder.setObjectMap(objectMapBuilder.build());
82107
}
83108
}

‎plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/response/document/common/DocumentFieldProtoUtils.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,4 +48,25 @@ public static ObjectMap.Value toProto(Object fieldValue) {
4848
return ObjectMapProtoUtils.toProto(fieldValue);
4949
}
5050

51+
/**
52+
* Converts a DocumentField values (list of objects) to its Protocol Buffer representation.
53+
* This method is equivalent to the {@link DocumentField#toXContent(XContentBuilder, ToXContent.Params)}
54+
*
55+
* @param fieldValues The list of DocumentField values to convert
56+
* @param valueBuilder The builder to populate with field values
57+
*/
58+
public static void toProto(List<Object> fieldValues, ObjectMap.Value.Builder valueBuilder) {
59+
ObjectMapProtoUtils.toProto(fieldValues, valueBuilder);
60+
}
61+
62+
/**
63+
* Converts a DocumentField value (object) to its Protocol Buffer representation.
64+
* This method is equivalent to the {@link DocumentField#toXContent(XContentBuilder, ToXContent.Params)}
65+
*
66+
* @param fieldValue The DocumentField value to convert
67+
* @param valueBuilder The builder to populate with field value
68+
*/
69+
public static void toProto(Object fieldValue, ObjectMap.Value.Builder valueBuilder) {
70+
ObjectMapProtoUtils.toProto(fieldValue, valueBuilder);
71+
}
5172
}

‎plugins/transport-grpc/src/main/java/org/opensearch/plugin/transport/grpc/proto/response/document/get/GetResultProtoUtils.java‎

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -35,26 +35,31 @@ private GetResultProtoUtils() {
3535
* This method is equivalent to the {@link GetResult#toXContent(XContentBuilder, ToXContent.Params)}
3636
*
3737
* @param getResult The GetResult to convert
38-
* @param responseItemBuilder
39-
* @return A Protocol Buffer InlineGetDictUserDefined representation
38+
* @param responseItemBuilder The builder to populate with GetResult data
39+
* @return The populated builder
4040
*/
4141
public static ResponseItem.Builder toProto(GetResult getResult, ResponseItem.Builder responseItemBuilder) {
42-
InlineGetDictUserDefined.Builder inlineGetDictUserDefinedBuilder = InlineGetDictUserDefined.newBuilder();
43-
42+
// Reuse the builder passed in by reference
4443
responseItemBuilder.setIndex(getResult.getIndex());
45-
responseItemBuilder.setId(ResponseItem.Id.newBuilder().setString(getResult.getId()).build());
44+
45+
// Avoid creating a new Id builder for each call
46+
ResponseItem.Id id = ResponseItem.Id.newBuilder().setString(getResult.getId()).build();
47+
responseItemBuilder.setId(id);
48+
49+
// Create the inline get dict builder only once
50+
InlineGetDictUserDefined.Builder inlineGetDictUserDefinedBuilder = InlineGetDictUserDefined.newBuilder();
4651

4752
if (getResult.isExists()) {
4853
// Set document version if available
4954
if (getResult.getVersion() != -1) {
5055
responseItemBuilder.setVersion(getResult.getVersion());
5156
}
52-
inlineGetDictUserDefinedBuilder = toProtoEmbedded(getResult, inlineGetDictUserDefinedBuilder);
57+
toProtoEmbedded(getResult, inlineGetDictUserDefinedBuilder);
5358
} else {
5459
inlineGetDictUserDefinedBuilder.setFound(false);
5560
}
5661

57-
responseItemBuilder.setGet(inlineGetDictUserDefinedBuilder);
62+
responseItemBuilder.setGet(inlineGetDictUserDefinedBuilder.build());
5863
return responseItemBuilder;
5964
}
6065

@@ -64,43 +69,42 @@ public static ResponseItem.Builder toProto(GetResult getResult, ResponseItem.Bui
6469
*
6570
* @param getResult The GetResult to convert
6671
* @param builder The builder to add the GetResult data to
67-
* @return The updated builder with the GetResult data
6872
*/
69-
public static InlineGetDictUserDefined.Builder toProtoEmbedded(GetResult getResult, InlineGetDictUserDefined.Builder builder) {
73+
public static void toProtoEmbedded(GetResult getResult, InlineGetDictUserDefined.Builder builder) {
7074
// Set sequence number and primary term if available
7175
if (getResult.getSeqNo() != SequenceNumbers.UNASSIGNED_SEQ_NO) {
7276
builder.setSeqNo(getResult.getSeqNo());
7377
builder.setPrimaryTerm(getResult.getPrimaryTerm());
7478
}
7579

76-
// TODO test output once GetDocument GRPC endpoint is implemented
77-
ObjectMap.Builder metadataFieldsBuilder = ObjectMap.newBuilder();
78-
for (DocumentField field : getResult.getMetadataFields().values()) {
79-
if (field.getName().equals(IgnoredFieldMapper.NAME)) {
80-
metadataFieldsBuilder.putFields(field.getName(), DocumentFieldProtoUtils.toProto(field.getValues()));
81-
} else {
82-
metadataFieldsBuilder.putFields(field.getName(), DocumentFieldProtoUtils.toProto(field.<Object>getValue()));
83-
}
84-
}
85-
builder.setMetadataFields(metadataFieldsBuilder.build());
86-
8780
// Set existence status
8881
builder.setFound(getResult.isExists());
8982

90-
// Set source if available
83+
// Set source if available - avoid unnecessary copying if possible
9184
if (getResult.source() != null) {
9285
builder.setSource(ByteString.copyFrom(getResult.source()));
9386
}
9487

95-
// TODO test output once GetDocument GRPC endpoint is implemented
96-
ObjectMap.Builder documentFieldsBuilder = ObjectMap.newBuilder();
88+
// Process metadata fields
89+
if (!getResult.getMetadataFields().isEmpty()) {
90+
ObjectMap.Builder metadataFieldsBuilder = ObjectMap.newBuilder();
91+
for (DocumentField field : getResult.getMetadataFields().values()) {
92+
if (field.getName().equals(IgnoredFieldMapper.NAME)) {
93+
metadataFieldsBuilder.putFields(field.getName(), DocumentFieldProtoUtils.toProto(field.getValues()));
94+
} else {
95+
metadataFieldsBuilder.putFields(field.getName(), DocumentFieldProtoUtils.toProto(field.<Object>getValue()));
96+
}
97+
}
98+
builder.setMetadataFields(metadataFieldsBuilder.build());
99+
}
100+
101+
// Process document fields - only create builder if needed
97102
if (!getResult.getDocumentFields().isEmpty()) {
103+
ObjectMap.Builder documentFieldsBuilder = ObjectMap.newBuilder();
98104
for (DocumentField field : getResult.getDocumentFields().values()) {
99105
documentFieldsBuilder.putFields(field.getName(), DocumentFieldProtoUtils.toProto(field.getValues()));
100106
}
107+
builder.setFields(documentFieldsBuilder.build());
101108
}
102-
builder.setFields(documentFieldsBuilder.build());
103-
104-
return builder;
105109
}
106110
}

0 commit comments

Comments
 (0)