From 0a03c542ec38fa53faa646cd8c782c8e02a7bfec Mon Sep 17 00:00:00 2001 From: Seunghyun Lee Date: Sat, 3 Feb 2024 16:47:59 -0800 Subject: [PATCH] Skip instead of throwing error on 'getValidDocIdMetadata' (#12360) * Skip instead of throwing error on 'getValidDocIdMetadata' - Current server side API throws the error if the snapshot file is not availble. In this case, we should skip instead of throwing the error. - Improve the API doc for validDocIds related APIs * Change the API 'validDocIdMetadata -> validDocIdsMetadata' --- ...Info.java => ValidDocIdsMetadataInfo.java} | 4 +- .../resources/PinotTableRestletResource.java | 19 ++--- .../util/ServerSegmentMetadataReader.java | 24 +++---- .../controller/util/TableMetadataReader.java | 10 +-- .../pinot/core/common/MinionConstants.java | 2 +- .../UpsertCompactionTaskGenerator.java | 26 +++---- .../UpsertCompactionTaskGeneratorTest.java | 30 ++++---- .../server/api/resources/TablesResource.java | 69 ++++++++++--------- .../pinot/server/api/TablesResourceTest.java | 21 +++--- 9 files changed, 107 insertions(+), 98 deletions(-) rename pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/{ValidDocIdMetadataInfo.java => ValidDocIdsMetadataInfo.java} (94%) diff --git a/pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/ValidDocIdMetadataInfo.java b/pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/ValidDocIdsMetadataInfo.java similarity index 94% rename from pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/ValidDocIdMetadataInfo.java rename to pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/ValidDocIdsMetadataInfo.java index ddec1df4dbe4..ce54424d16ed 100644 --- a/pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/ValidDocIdMetadataInfo.java +++ b/pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/ValidDocIdsMetadataInfo.java @@ -23,7 +23,7 @@ @JsonIgnoreProperties(ignoreUnknown = true) -public class ValidDocIdMetadataInfo { +public class ValidDocIdsMetadataInfo { private final String _segmentName; private final long _totalValidDocs; private final long _totalInvalidDocs; @@ -31,7 +31,7 @@ public class ValidDocIdMetadataInfo { private final String _segmentCrc; private final ValidDocIdsType _validDocIdsType; - public ValidDocIdMetadataInfo(@JsonProperty("segmentName") String segmentName, + public ValidDocIdsMetadataInfo(@JsonProperty("segmentName") String segmentName, @JsonProperty("totalValidDocs") long totalValidDocs, @JsonProperty("totalInvalidDocs") long totalInvalidDocs, @JsonProperty("totalDocs") long totalDocs, @JsonProperty("segmentCrc") String segmentCrc, @JsonProperty("validDocIdsType") ValidDocIdsType validDocIdsType) { diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java index 4505ff0bc4ab..aedc6a7ef896 100644 --- a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java +++ b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java @@ -80,6 +80,7 @@ import org.apache.pinot.common.metrics.ControllerMetrics; import org.apache.pinot.common.response.server.TableIndexMetadataResponse; import org.apache.pinot.common.restlet.resources.TableSegmentValidationInfo; +import org.apache.pinot.common.restlet.resources.ValidDocIdsType; import org.apache.pinot.common.utils.helix.HelixHelper; import org.apache.pinot.controller.ControllerConf; import org.apache.pinot.controller.api.access.AccessControlFactory; @@ -951,18 +952,18 @@ public String getTableAggregateMetadata( } @GET - @Path("tables/{tableName}/validDocIdMetadata") + @Path("tables/{tableName}/validDocIdsMetadata") @Authorize(targetType = TargetType.TABLE, paramName = "tableName", action = Actions.Table.GET_METADATA) @Produces(MediaType.APPLICATION_JSON) @ApiOperation(value = "Get the aggregate valid doc id metadata of all segments for a table", notes = "Get the " + "aggregate valid doc id metadata of all segments for a table") - public String getTableAggregateValidDocIdMetadata( + public String getTableAggregateValidDocIdsMetadata( @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName, @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr, @ApiParam(value = "A list of segments", allowMultiple = true) @QueryParam("segmentNames") List segmentNames, - @ApiParam(value = "Valid doc id type", example = "SNAPSHOT|IN_MEMORY|IN_MEMORY_WITH_DELETE") - @QueryParam("validDocIdsType") String validDocIdsType) { + @ApiParam(value = "Valid doc ids type") + @QueryParam("validDocIdsType") ValidDocIdsType validDocIdsType) { LOGGER.info("Received a request to fetch aggregate valid doc id metadata for a table {}", tableName); TableType tableType = Constants.validateTableType(tableTypeStr); if (tableType == TableType.OFFLINE) { @@ -972,21 +973,21 @@ public String getTableAggregateValidDocIdMetadata( String tableNameWithType = ResourceUtils.getExistingTableNamesWithType(_pinotHelixResourceManager, tableName, tableType, LOGGER).get(0); - String validDocIdMetadata; + String validDocIdsMetadata; try { TableMetadataReader tableMetadataReader = new TableMetadataReader(_executor, _connectionManager, _pinotHelixResourceManager); JsonNode segmentsMetadataJson = - tableMetadataReader.getAggregateValidDocIdMetadata(tableNameWithType, segmentNames, validDocIdsType, - _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000); - validDocIdMetadata = JsonUtils.objectToPrettyString(segmentsMetadataJson); + tableMetadataReader.getAggregateValidDocIdsMetadata(tableNameWithType, segmentNames, + validDocIdsType.toString(), _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000); + validDocIdsMetadata = JsonUtils.objectToPrettyString(segmentsMetadataJson); } catch (InvalidConfigException e) { throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.BAD_REQUEST); } catch (IOException ioe) { throw new ControllerApplicationException(LOGGER, "Error parsing Pinot server response: " + ioe.getMessage(), Response.Status.INTERNAL_SERVER_ERROR, ioe); } - return validDocIdMetadata; + return validDocIdsMetadata; } @GET diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/util/ServerSegmentMetadataReader.java b/pinot-controller/src/main/java/org/apache/pinot/controller/util/ServerSegmentMetadataReader.java index fad0559b7639..f728d5163520 100644 --- a/pinot-controller/src/main/java/org/apache/pinot/controller/util/ServerSegmentMetadataReader.java +++ b/pinot-controller/src/main/java/org/apache/pinot/controller/util/ServerSegmentMetadataReader.java @@ -42,8 +42,8 @@ import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import org.apache.pinot.common.restlet.resources.TableMetadataInfo; import org.apache.pinot.common.restlet.resources.TableSegments; -import org.apache.pinot.common.restlet.resources.ValidDocIdMetadataInfo; import org.apache.pinot.common.restlet.resources.ValidDocIdsBitmapResponse; +import org.apache.pinot.common.restlet.resources.ValidDocIdsMetadataInfo; import org.apache.pinot.common.utils.RoaringBitmapUtils; import org.apache.pinot.spi.utils.JsonUtils; import org.glassfish.jersey.client.ClientConfig; @@ -209,7 +209,7 @@ public List getSegmentMetadataFromServer(String tableNameWithType, * * @return segment metadata as a JSON string */ - public List getValidDocIdMetadataFromServer(String tableNameWithType, + public List getValidDocIdsMetadataFromServer(String tableNameWithType, Map> serverToSegmentsMap, BiMap serverToEndpoints, @Nullable List segmentNames, int timeoutMs, String validDocIdsType) { List> serverURLsAndBodies = new ArrayList<>(); @@ -226,7 +226,7 @@ public List getValidDocIdMetadataFromServer(String table } } } - serverURLsAndBodies.add(generateValidDocIdMetadataURL(tableNameWithType, segmentsToQuery, validDocIdsType, + serverURLsAndBodies.add(generateValidDocIdsMetadataURL(tableNameWithType, segmentsToQuery, validDocIdsType, serverToEndpoints.get(serverToSegments.getKey()))); } @@ -239,16 +239,16 @@ public List getValidDocIdMetadataFromServer(String table completionServiceHelper.doMultiPostRequest(serverURLsAndBodies, tableNameWithType, false, requestHeaders, timeoutMs, null); - List validDocIdMetadataInfos = new ArrayList<>(); + List validDocIdsMetadataInfos = new ArrayList<>(); int failedParses = 0; int returnedSegmentsCount = 0; for (Map.Entry streamResponse : serviceResponse._httpResponses.entrySet()) { try { - String validDocIdMetadataList = streamResponse.getValue(); - List validDocIdMetadataInfo = - JsonUtils.stringToObject(validDocIdMetadataList, new TypeReference>() { + String validDocIdsMetadataList = streamResponse.getValue(); + List validDocIdsMetadataInfo = + JsonUtils.stringToObject(validDocIdsMetadataList, new TypeReference>() { }); - validDocIdMetadataInfos.addAll(validDocIdMetadataInfo); + validDocIdsMetadataInfos.addAll(validDocIdsMetadataInfo); returnedSegmentsCount++; } catch (Exception e) { failedParses++; @@ -261,12 +261,12 @@ public List getValidDocIdMetadataFromServer(String table } if (segmentNames != null && returnedSegmentsCount != segmentNames.size()) { - LOGGER.error("Unable to get validDocIdMetadata from all servers. Expected: {}, Actual: {}", segmentNames.size(), + LOGGER.error("Unable to get validDocIdsMetadata from all servers. Expected: {}, Actual: {}", segmentNames.size(), returnedSegmentsCount); } LOGGER.info("Retrieved valid doc id metadata for {} segments from {} servers.", returnedSegmentsCount, serverURLsAndBodies.size()); - return validDocIdMetadataInfos; + return validDocIdsMetadataInfos; } /** @@ -354,7 +354,7 @@ private String generateValidDocIdsBitmapURL(String tableNameWithType, String seg return url; } - private Pair generateValidDocIdMetadataURL(String tableNameWithType, List segmentNames, + private Pair generateValidDocIdsMetadataURL(String tableNameWithType, List segmentNames, String validDocIdsType, String endpoint) { tableNameWithType = URLEncoder.encode(tableNameWithType, StandardCharsets.UTF_8); TableSegments tableSegments = new TableSegments(segmentNames); @@ -365,7 +365,7 @@ private Pair generateValidDocIdMetadataURL(String tableNameWithT LOGGER.error("Failed to convert segment names to json request body: segmentNames={}", segmentNames); throw new RuntimeException(e); } - String url = String.format("%s/tables/%s/validDocIdMetadata", endpoint, tableNameWithType); + String url = String.format("%s/tables/%s/validDocIdsMetadata", endpoint, tableNameWithType); if (validDocIdsType != null) { url = url + "?validDocIdsType=" + validDocIdsType; } diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/util/TableMetadataReader.java b/pinot-controller/src/main/java/org/apache/pinot/controller/util/TableMetadataReader.java index 25ca6fc7f9fa..389c8d2e9425 100644 --- a/pinot-controller/src/main/java/org/apache/pinot/controller/util/TableMetadataReader.java +++ b/pinot-controller/src/main/java/org/apache/pinot/controller/util/TableMetadataReader.java @@ -31,7 +31,7 @@ import org.apache.http.conn.HttpClientConnectionManager; import org.apache.pinot.common.exception.InvalidConfigException; import org.apache.pinot.common.restlet.resources.TableMetadataInfo; -import org.apache.pinot.common.restlet.resources.ValidDocIdMetadataInfo; +import org.apache.pinot.common.restlet.resources.ValidDocIdsMetadataInfo; import org.apache.pinot.controller.helix.core.PinotHelixResourceManager; import org.apache.pinot.spi.utils.JsonUtils; @@ -156,9 +156,9 @@ public JsonNode getAggregateTableMetadata(String tableNameWithType, List /** * This method retrieves the aggregated valid doc id metadata for a given table. - * @return a list of ValidDocIdMetadataInfo + * @return a list of ValidDocIdsMetadataInfo */ - public JsonNode getAggregateValidDocIdMetadata(String tableNameWithType, List segmentNames, + public JsonNode getAggregateValidDocIdsMetadata(String tableNameWithType, List segmentNames, String validDocIdsType, int timeoutMs) throws InvalidConfigException { final Map> serverToSegments = @@ -168,8 +168,8 @@ public JsonNode getAggregateValidDocIdMetadata(String tableNameWithType, List aggregateTableMetadataInfo = - serverSegmentMetadataReader.getValidDocIdMetadataFromServer(tableNameWithType, serverToSegments, endpoints, + List aggregateTableMetadataInfo = + serverSegmentMetadataReader.getValidDocIdsMetadataFromServer(tableNameWithType, serverToSegments, endpoints, segmentNames, timeoutMs, validDocIdsType); return JsonUtils.objectToJsonNode(aggregateTableMetadataInfo); } diff --git a/pinot-core/src/main/java/org/apache/pinot/core/common/MinionConstants.java b/pinot-core/src/main/java/org/apache/pinot/core/common/MinionConstants.java index a02b2876e38b..02f00046b94b 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/common/MinionConstants.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/common/MinionConstants.java @@ -166,7 +166,7 @@ public static class UpsertCompactionTask { public static final String INVALID_RECORDS_THRESHOLD_COUNT = "invalidRecordsThresholdCount"; /** - * Valid doc id type + * Valid doc ids type */ public static final String VALID_DOC_IDS_TYPE = "validDocIdsType"; } diff --git a/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskGenerator.java b/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskGenerator.java index ad005a1ca2c8..6d21d8417fc8 100644 --- a/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskGenerator.java +++ b/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskGenerator.java @@ -31,7 +31,7 @@ import org.apache.helix.task.TaskState; import org.apache.pinot.common.exception.InvalidConfigException; import org.apache.pinot.common.metadata.segment.SegmentZKMetadata; -import org.apache.pinot.common.restlet.resources.ValidDocIdMetadataInfo; +import org.apache.pinot.common.restlet.resources.ValidDocIdsMetadataInfo; import org.apache.pinot.common.restlet.resources.ValidDocIdsType; import org.apache.pinot.controller.helix.core.PinotHelixResourceManager; import org.apache.pinot.controller.helix.core.minion.generator.BaseTaskGenerator; @@ -151,15 +151,15 @@ public List generateTasks(List tableConfigs) { validDocIdsType)); } - List validDocIdMetadataList = - serverSegmentMetadataReader.getValidDocIdMetadataFromServer(tableNameWithType, serverToSegments, + List validDocIdsMetadataList = + serverSegmentMetadataReader.getValidDocIdsMetadataFromServer(tableNameWithType, serverToSegments, serverToEndpoints, null, 60_000, validDocIdsType.toString()); Map completedSegmentsMap = completedSegments.stream().collect(Collectors.toMap(SegmentZKMetadata::getSegmentName, Function.identity())); SegmentSelectionResult segmentSelectionResult = - processValidDocIdMetadata(taskConfigs, completedSegmentsMap, validDocIdMetadataList); + processValidDocIdsMetadata(taskConfigs, completedSegmentsMap, validDocIdsMetadataList); if (!segmentSelectionResult.getSegmentsForDeletion().isEmpty()) { pinotHelixResourceManager.deleteSegments(tableNameWithType, segmentSelectionResult.getSegmentsForDeletion(), @@ -195,8 +195,8 @@ public List generateTasks(List tableConfigs) { } @VisibleForTesting - public static SegmentSelectionResult processValidDocIdMetadata(Map taskConfigs, - Map completedSegmentsMap, List validDocIdMetadataInfoList) { + public static SegmentSelectionResult processValidDocIdsMetadata(Map taskConfigs, + Map completedSegmentsMap, List validDocIdsMetadataInfoList) { double invalidRecordsThresholdPercent = Double.parseDouble( taskConfigs.getOrDefault(UpsertCompactionTask.INVALID_RECORDS_THRESHOLD_PERCENT, String.valueOf(DEFAULT_INVALID_RECORDS_THRESHOLD_PERCENT))); @@ -205,19 +205,19 @@ public static SegmentSelectionResult processValidDocIdMetadata(Map segmentsForCompaction = new ArrayList<>(); List segmentsForDeletion = new ArrayList<>(); - for (ValidDocIdMetadataInfo validDocIdMetadata : validDocIdMetadataInfoList) { - long totalInvalidDocs = validDocIdMetadata.getTotalInvalidDocs(); - String segmentName = validDocIdMetadata.getSegmentName(); + for (ValidDocIdsMetadataInfo validDocIdsMetadata : validDocIdsMetadataInfoList) { + long totalInvalidDocs = validDocIdsMetadata.getTotalInvalidDocs(); + String segmentName = validDocIdsMetadata.getSegmentName(); // Skip segments if the crc from zk metadata and server does not match. They may be being reloaded. SegmentZKMetadata segment = completedSegmentsMap.get(segmentName); - if (segment.getCrc() != Long.parseLong(validDocIdMetadata.getSegmentCrc())) { + if (segment.getCrc() != Long.parseLong(validDocIdsMetadata.getSegmentCrc())) { LOGGER.warn( - "CRC mismatch for segment: {}, skipping it for compaction (segmentZKMetadata={}, validDocIdMetadata={})", - segmentName, segment.getCrc(), validDocIdMetadata.getSegmentCrc()); + "CRC mismatch for segment: {}, skipping it for compaction (segmentZKMetadata={}, validDocIdsMetadata={})", + segmentName, segment.getCrc(), validDocIdsMetadata.getSegmentCrc()); continue; } - long totalDocs = validDocIdMetadata.getTotalDocs(); + long totalDocs = validDocIdsMetadata.getTotalDocs(); double invalidRecordPercent = ((double) totalInvalidDocs / totalDocs) * 100; if (totalInvalidDocs == totalDocs) { segmentsForDeletion.add(segment.getSegmentName()); diff --git a/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/test/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskGeneratorTest.java b/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/test/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskGeneratorTest.java index d30107056bc6..d74b03e81580 100644 --- a/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/test/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskGeneratorTest.java +++ b/pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/test/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskGeneratorTest.java @@ -28,7 +28,7 @@ import java.util.concurrent.TimeUnit; import org.apache.helix.model.IdealState; import org.apache.pinot.common.metadata.segment.SegmentZKMetadata; -import org.apache.pinot.common.restlet.resources.ValidDocIdMetadataInfo; +import org.apache.pinot.common.restlet.resources.ValidDocIdsMetadataInfo; import org.apache.pinot.controller.helix.core.minion.ClusterInfoAccessor; import org.apache.pinot.core.common.MinionConstants; import org.apache.pinot.core.common.MinionConstants.UpsertCompactionTask; @@ -182,7 +182,7 @@ public void testGetMaxTasks() { } @Test - public void testProcessValidDocIdMetadata() + public void testProcessValidDocIdsMetadata() throws IOException { Map compactionConfigs = getCompactionConfigs("1", "10"); String json = "[{" + "\"totalValidDocs\" : 50," + "\"totalInvalidDocs\" : 50," + "\"segmentName\" : \"" @@ -190,12 +190,12 @@ public void testProcessValidDocIdMetadata() + _completedSegment.getCrc() + "\"}," + "{" + "\"totalValidDocs\" : 0," + "\"totalInvalidDocs\" : 10," + "\"segmentName\" : \"" + _completedSegment2.getSegmentName() + "\", " + "\"segmentCrc\" : \"" + _completedSegment2.getCrc() + "\"," + "\"totalDocs\" : 10" + "}]"; - List validDocIdMetadataInfo = - JsonUtils.stringToObject(json, new TypeReference>() { + List validDocIdsMetadataInfo = + JsonUtils.stringToObject(json, new TypeReference>() { }); UpsertCompactionTaskGenerator.SegmentSelectionResult segmentSelectionResult = - UpsertCompactionTaskGenerator.processValidDocIdMetadata(compactionConfigs, _completedSegmentsMap, - validDocIdMetadataInfo); + UpsertCompactionTaskGenerator.processValidDocIdsMetadata(compactionConfigs, _completedSegmentsMap, + validDocIdsMetadataInfo); assertEquals(segmentSelectionResult.getSegmentsForCompaction().get(0).getSegmentName(), _completedSegment.getSegmentName()); assertEquals(segmentSelectionResult.getSegmentsForDeletion().get(0), _completedSegment2.getSegmentName()); @@ -203,23 +203,23 @@ public void testProcessValidDocIdMetadata() // test with a higher invalidRecordsThresholdPercent compactionConfigs = getCompactionConfigs("60", "10"); segmentSelectionResult = - UpsertCompactionTaskGenerator.processValidDocIdMetadata(compactionConfigs, _completedSegmentsMap, - validDocIdMetadataInfo); + UpsertCompactionTaskGenerator.processValidDocIdsMetadata(compactionConfigs, _completedSegmentsMap, + validDocIdsMetadataInfo); assertTrue(segmentSelectionResult.getSegmentsForCompaction().isEmpty()); // test without an invalidRecordsThresholdPercent compactionConfigs = getCompactionConfigs("0", "10"); segmentSelectionResult = - UpsertCompactionTaskGenerator.processValidDocIdMetadata(compactionConfigs, _completedSegmentsMap, - validDocIdMetadataInfo); + UpsertCompactionTaskGenerator.processValidDocIdsMetadata(compactionConfigs, _completedSegmentsMap, + validDocIdsMetadataInfo); assertEquals(segmentSelectionResult.getSegmentsForCompaction().get(0).getSegmentName(), _completedSegment.getSegmentName()); // test without a invalidRecordsThresholdCount compactionConfigs = getCompactionConfigs("30", "0"); segmentSelectionResult = - UpsertCompactionTaskGenerator.processValidDocIdMetadata(compactionConfigs, _completedSegmentsMap, - validDocIdMetadataInfo); + UpsertCompactionTaskGenerator.processValidDocIdsMetadata(compactionConfigs, _completedSegmentsMap, + validDocIdsMetadataInfo); assertEquals(segmentSelectionResult.getSegmentsForCompaction().get(0).getSegmentName(), _completedSegment.getSegmentName()); @@ -230,11 +230,11 @@ public void testProcessValidDocIdMetadata() + "\"segmentName\" : \"" + _completedSegment2.getSegmentName() + "\", " + "\"segmentCrc\" : \"" + _completedSegment2.getCrc() + "\"," + "\"totalDocs\" : 10" + "}]"; - validDocIdMetadataInfo = JsonUtils.stringToObject(json, new TypeReference>() { + validDocIdsMetadataInfo = JsonUtils.stringToObject(json, new TypeReference>() { }); segmentSelectionResult = - UpsertCompactionTaskGenerator.processValidDocIdMetadata(compactionConfigs, _completedSegmentsMap, - validDocIdMetadataInfo); + UpsertCompactionTaskGenerator.processValidDocIdsMetadata(compactionConfigs, _completedSegmentsMap, + validDocIdsMetadataInfo); // completedSegment is supposed to be filtered out Assert.assertEquals(segmentSelectionResult.getSegmentsForCompaction().size(), 0); diff --git a/pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java b/pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java index 52702c831fa1..5e458bbb289b 100644 --- a/pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java +++ b/pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java @@ -476,7 +476,7 @@ public Response downloadSegment( public ValidDocIdsBitmapResponse downloadValidDocIdsBitmap( @ApiParam(value = "Name of the table with type REALTIME", required = true, example = "myTable_REALTIME") @PathParam("tableNameWithType") String tableNameWithType, - @ApiParam(value = "Valid doc id type", example = "SNAPSHOT|IN_MEMORY|IN_MEMORY_WITH_DELETE") + @ApiParam(value = "Valid doc ids type") @QueryParam("validDocIdsType") String validDocIdsType, @ApiParam(value = "Name of the segment", required = true) @PathParam("segmentName") @Encoded String segmentName, @Context HttpHeaders httpHeaders) { @@ -508,8 +508,10 @@ public ValidDocIdsBitmapResponse downloadValidDocIdsBitmap( MutableRoaringBitmap validDocIdSnapshot = validDocIdsSnapshotPair.getRight(); if (validDocIdSnapshot == null) { - String msg = String.format("Missing validDocIds for table %s segment %s does not exist", tableNameWithType, - segmentDataManager.getSegmentName()); + String msg = String.format( + "Found that validDocIds is missing while fetching validDocIds for table %s segment %s while " + + "reading the validDocIds with validDocIdType %s", + tableNameWithType, segmentDataManager.getSegmentName(), validDocIdsType); LOGGER.warn(msg); throw new WebApplicationException(msg, Response.Status.NOT_FOUND); } @@ -536,7 +538,7 @@ public Response downloadValidDocIds( @ApiParam(value = "Name of the table with type REALTIME", required = true, example = "myTable_REALTIME") @PathParam("tableNameWithType") String tableNameWithType, @ApiParam(value = "Name of the segment", required = true) @PathParam("segmentName") @Encoded String segmentName, - @ApiParam(value = "Valid doc id type", example = "SNAPSHOT|IN_MEMORY|IN_MEMORY_WITH_DELETE") + @ApiParam(value = "Valid doc ids type") @QueryParam("validDocIdsType") String validDocIdsType, @Context HttpHeaders httpHeaders) { segmentName = URIUtils.decode(segmentName); LOGGER.info("Received a request to download validDocIds for segment {} table {}", segmentName, tableNameWithType); @@ -564,8 +566,10 @@ public Response downloadValidDocIds( getValidDocIds(indexSegment, validDocIdsType); MutableRoaringBitmap validDocIdSnapshot = validDocIdSnapshotPair.getRight(); if (validDocIdSnapshot == null) { - String msg = String.format("Missing validDocIds for table %s segment %s does not exist", tableNameWithType, - segmentDataManager.getSegmentName()); + String msg = String.format( + "Found that validDocIds is missing while fetching validDocIds for table %s segment %s while " + + "reading the validDocIds with validDocIdType %s", + tableNameWithType, segmentDataManager.getSegmentName(), validDocIdsType); LOGGER.warn(msg); throw new WebApplicationException(msg, Response.Status.NOT_FOUND); } @@ -579,6 +583,7 @@ public Response downloadValidDocIds( } } + @Deprecated @GET @Path("/tables/{tableNameWithType}/validDocIdMetadata") @Produces(MediaType.APPLICATION_JSON) @@ -588,36 +593,36 @@ public Response downloadValidDocIds( response = ErrorInfo.class), @ApiResponse(code = 404, message = "Table or segment not found", response = ErrorInfo.class) }) - public String getValidDocIdMetadata( + public String getValidDocIdsMetadata( @ApiParam(value = "Table name including type", required = true, example = "myTable_REALTIME") @PathParam("tableNameWithType") String tableNameWithType, - @ApiParam(value = "Valid doc id type", example = "SNAPSHOT|IN_MEMORY|IN_MEMORY_WITH_DELETE") + @ApiParam(value = "Valid doc ids type") @QueryParam("validDocIdsType") String validDocIdsType, @ApiParam(value = "Segment name", allowMultiple = true) @QueryParam("segmentNames") List segmentNames) { return ResourceUtils.convertToJsonString( - processValidDocIdMetadata(tableNameWithType, segmentNames, validDocIdsType)); + processValidDocIdsMetadata(tableNameWithType, segmentNames, validDocIdsType)); } @POST - @Path("/tables/{tableNameWithType}/validDocIdMetadata") + @Path("/tables/{tableNameWithType}/validDocIdsMetadata") @Produces(MediaType.APPLICATION_JSON) - @ApiOperation(value = "Provides segment validDocId metadata", notes = "Provides segment validDocId metadata") + @ApiOperation(value = "Provides segment valid doc ids metadata", notes = "Provides segment valid doc ids metadata") @ApiResponses(value = { @ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 500, message = "Internal server error", response = ErrorInfo.class), @ApiResponse(code = 404, message = "Table or segment not found", response = ErrorInfo.class) }) - public String getValidDocIdMetadata( + public String getValidDocIdsMetadata( @ApiParam(value = "Table name including type", required = true, example = "myTable_REALTIME") @PathParam("tableNameWithType") String tableNameWithType, - @ApiParam(value = "Valid doc id type", example = "SNAPSHOT|IN_MEMORY|IN_MEMORY_WITH_DELETE") + @ApiParam(value = "Valid doc ids type") @QueryParam("validDocIdsType") String validDocIdsType, TableSegments tableSegments) { List segmentNames = tableSegments.getSegments(); return ResourceUtils.convertToJsonString( - processValidDocIdMetadata(tableNameWithType, segmentNames, validDocIdsType)); + processValidDocIdsMetadata(tableNameWithType, segmentNames, validDocIdsType)); } - private List> processValidDocIdMetadata(String tableNameWithType, List segments, + private List> processValidDocIdsMetadata(String tableNameWithType, List segments, String validDocIdsType) { TableDataManager tableDataManager = ServerResourceUtils.checkGetTableDataManager(_serverInstance, tableNameWithType); @@ -633,7 +638,7 @@ private List> processValidDocIdMetadata(String tableNameWith Response.Status.NOT_FOUND); } } - List> allValidDocIdMetadata = new ArrayList<>(); + List> allValidDocIdsMetadata = new ArrayList<>(); for (SegmentDataManager segmentDataManager : segmentDataManagers) { try { IndexSegment indexSegment = segmentDataManager.getSegment(); @@ -652,30 +657,32 @@ private List> processValidDocIdMetadata(String tableNameWith final Pair validDocIdSnapshotPair = getValidDocIds(indexSegment, validDocIdsType); String finalValidDocIdsType = validDocIdSnapshotPair.getLeft().toString(); - MutableRoaringBitmap validDocIdSnapshot = validDocIdSnapshotPair.getRight(); - if (validDocIdSnapshot == null) { - String msg = String.format("Missing validDocIds for table %s segment %s does not exist", tableNameWithType, - segmentDataManager.getSegmentName()); + MutableRoaringBitmap validDocIdsSnapshot = validDocIdSnapshotPair.getRight(); + if (validDocIdsSnapshot == null) { + String msg = String.format( + "Found that validDocIds is missing while processing validDocIdsMetadata for table %s segment %s while " + + "reading the validDocIds with validDocIdType %s", + tableNameWithType, segmentDataManager.getSegmentName(), validDocIdsType); LOGGER.warn(msg); - throw new WebApplicationException(msg, Response.Status.NOT_FOUND); + continue; } - Map validDocIdMetadata = new HashMap<>(); + Map validDocIdsMetadata = new HashMap<>(); int totalDocs = indexSegment.getSegmentMetadata().getTotalDocs(); - int totalValidDocs = validDocIdSnapshot.getCardinality(); + int totalValidDocs = validDocIdsSnapshot.getCardinality(); int totalInvalidDocs = totalDocs - totalValidDocs; - validDocIdMetadata.put("segmentName", segmentDataManager.getSegmentName()); - validDocIdMetadata.put("totalDocs", totalDocs); - validDocIdMetadata.put("totalValidDocs", totalValidDocs); - validDocIdMetadata.put("totalInvalidDocs", totalInvalidDocs); - validDocIdMetadata.put("segmentCrc", indexSegment.getSegmentMetadata().getCrc()); - validDocIdMetadata.put("validDocIdsType", finalValidDocIdsType); - allValidDocIdMetadata.add(validDocIdMetadata); + validDocIdsMetadata.put("segmentName", segmentDataManager.getSegmentName()); + validDocIdsMetadata.put("totalDocs", totalDocs); + validDocIdsMetadata.put("totalValidDocs", totalValidDocs); + validDocIdsMetadata.put("totalInvalidDocs", totalInvalidDocs); + validDocIdsMetadata.put("segmentCrc", indexSegment.getSegmentMetadata().getCrc()); + validDocIdsMetadata.put("validDocIdsType", finalValidDocIdsType); + allValidDocIdsMetadata.add(validDocIdsMetadata); } finally { tableDataManager.releaseSegment(segmentDataManager); } } - return allValidDocIdMetadata; + return allValidDocIdsMetadata; } private Pair getValidDocIds(IndexSegment indexSegment, diff --git a/pinot-server/src/test/java/org/apache/pinot/server/api/TablesResourceTest.java b/pinot-server/src/test/java/org/apache/pinot/server/api/TablesResourceTest.java index bca644fb16e8..e4db9cd41908 100644 --- a/pinot-server/src/test/java/org/apache/pinot/server/api/TablesResourceTest.java +++ b/pinot-server/src/test/java/org/apache/pinot/server/api/TablesResourceTest.java @@ -298,6 +298,7 @@ public void testDownloadValidDocIdsSnapshot() Assert.assertEquals(response.getStatus(), Response.Status.NOT_FOUND.getStatusCode()); } + @Deprecated @Test public void testValidDocIdMetadata() throws IOException { @@ -323,7 +324,7 @@ public void testValidDocIdMetadata() } @Test - public void testValidDocIdMetadataPost() + public void testValidDocIdsMetadataPost() throws IOException { IndexSegment segment = _realtimeIndexSegments.get(0); // Verify the content of the downloaded snapshot from a realtime table. @@ -334,18 +335,18 @@ public void testValidDocIdMetadataPost() List segments = List.of(segment.getSegmentName()); TableSegments tableSegments = new TableSegments(segments); - String validDocIdMetadataPath = - "/tables/" + TableNameBuilder.REALTIME.tableNameWithType(TABLE_NAME) + "/validDocIdMetadata"; + String validDocIdsMetadataPath = + "/tables/" + TableNameBuilder.REALTIME.tableNameWithType(TABLE_NAME) + "/validDocIdsMetadata"; String response = - _webTarget.path(validDocIdMetadataPath).queryParam("segmentNames", segment.getSegmentName()).request() + _webTarget.path(validDocIdsMetadataPath).queryParam("segmentNames", segment.getSegmentName()).request() .post(Entity.json(tableSegments), String.class); - JsonNode validDocIdMetadata = JsonUtils.stringToJsonNode(response).get(0); + JsonNode validDocIdsMetadata = JsonUtils.stringToJsonNode(response).get(0); - Assert.assertEquals(validDocIdMetadata.get("totalDocs").asInt(), 100000); - Assert.assertEquals(validDocIdMetadata.get("totalValidDocs").asInt(), 8); - Assert.assertEquals(validDocIdMetadata.get("totalInvalidDocs").asInt(), 99992); - Assert.assertEquals(validDocIdMetadata.get("segmentCrc").asText(), "1265679343"); - Assert.assertEquals(validDocIdMetadata.get("validDocIdsType").asText(), "SNAPSHOT"); + Assert.assertEquals(validDocIdsMetadata.get("totalDocs").asInt(), 100000); + Assert.assertEquals(validDocIdsMetadata.get("totalValidDocs").asInt(), 8); + Assert.assertEquals(validDocIdsMetadata.get("totalInvalidDocs").asInt(), 99992); + Assert.assertEquals(validDocIdsMetadata.get("segmentCrc").asText(), "1265679343"); + Assert.assertEquals(validDocIdsMetadata.get("validDocIdsType").asText(), "SNAPSHOT"); } // Verify metadata file from segments.