Jackie-Jiang commented on code in PR #16094:
URL: https://github.com/apache/pinot/pull/16094#discussion_r2205638255
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/json/BaseJsonIndexCreator.java:
##########
@@ -91,7 +101,51 @@ public abstract class BaseJsonIndexCreator implements
JsonIndexCreator {
@Override
public void add(String jsonString)
throws IOException {
- addFlattenedRecords(JsonUtils.flatten(jsonString, _jsonIndexConfig));
+ List<Map<String, String>> flattenedRecord;
+ try {
+ flattenedRecord = JsonUtils.flatten(jsonString, _jsonIndexConfig);
+ if (flattenedRecord.equals(JsonUtils.SKIPPED_FLATTENED_RECORD)) {
Review Comment:
Let's do reference check here to lower the overhead
```suggestion
if (flattenedRecord == JsonUtils.SKIPPED_FLATTENED_RECORD) {
```
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/json/BaseJsonIndexCreator.java:
##########
@@ -91,7 +101,51 @@ public abstract class BaseJsonIndexCreator implements
JsonIndexCreator {
@Override
public void add(String jsonString)
throws IOException {
- addFlattenedRecords(JsonUtils.flatten(jsonString, _jsonIndexConfig));
+ List<Map<String, String>> flattenedRecord;
+ try {
+ flattenedRecord = JsonUtils.flatten(jsonString, _jsonIndexConfig);
+ if (flattenedRecord.equals(JsonUtils.SKIPPED_FLATTENED_RECORD)) {
+ // The default SKIPPED_FLATTENED_RECORD was returned, this can only
happen if the original record could not be
+ // flattened, update the metric
+ String metricKeyName =
+ _tableNameWithType + "-" +
JsonIndexType.INDEX_DISPLAY_NAME.toUpperCase(Locale.US) + "-indexingError";
+ ServerMetrics.get().addMeteredTableValue(metricKeyName,
ServerMeter.INDEXING_FAILURES, 1);
+ }
+ } catch (Exception e) {
+ if (_continueOnError) {
+ // Caught exception while trying to add, update metric and add a
default SKIPPED_FLATTENED_RECORD
+ // This check is needed in the case where
`_jsonIndexConfig.getSkipInvalidJson()` is false,
+ // but _continueOnError is true
+ String metricKeyName =
+ _tableNameWithType + "-" +
JsonIndexType.INDEX_DISPLAY_NAME.toUpperCase(Locale.US) + "-indexingError";
+ ServerMetrics.get().addMeteredTableValue(metricKeyName,
ServerMeter.INDEXING_FAILURES, 1);
+ flattenedRecord = JsonUtils.SKIPPED_FLATTENED_RECORD;
+ } else {
+ throw e;
+ }
+ }
+ addFlattenedRecords(flattenedRecord);
+ }
+
+ @Override
+ public void add(Map value)
+ throws IOException {
+ String valueToAdd;
+ try {
+ valueToAdd = JsonUtils.objectToString(value);
Review Comment:
Let's add a TODO to avoid this ser/de from map -> string -> json node. We
can address it separately
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/json/BaseJsonIndexCreator.java:
##########
@@ -91,7 +101,51 @@ public abstract class BaseJsonIndexCreator implements
JsonIndexCreator {
@Override
public void add(String jsonString)
throws IOException {
- addFlattenedRecords(JsonUtils.flatten(jsonString, _jsonIndexConfig));
+ List<Map<String, String>> flattenedRecord;
+ try {
+ flattenedRecord = JsonUtils.flatten(jsonString, _jsonIndexConfig);
+ if (flattenedRecord.equals(JsonUtils.SKIPPED_FLATTENED_RECORD)) {
+ // The default SKIPPED_FLATTENED_RECORD was returned, this can only
happen if the original record could not be
+ // flattened, update the metric
+ String metricKeyName =
+ _tableNameWithType + "-" +
JsonIndexType.INDEX_DISPLAY_NAME.toUpperCase(Locale.US) + "-indexingError";
+ ServerMetrics.get().addMeteredTableValue(metricKeyName,
ServerMeter.INDEXING_FAILURES, 1);
Review Comment:
This can be extracted as a helper method. Same for other places
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/geospatial/BaseH3IndexCreator.java:
##########
@@ -116,13 +120,16 @@ public Geometry deserialize(byte[] bytes) {
@Override
public void add(@Nullable Geometry geometry)
throws IOException {
- if (geometry == null || !(geometry instanceof Point)) {
+ if (_continueOnError && (geometry == null || !(geometry instanceof
Point))) {
Review Comment:
(minor) Can be simplified
```suggestion
if (_continueOnError && !(geometry instanceof Point)) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
