You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2020/10/23 02:12:32 UTC
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6172: Adding a new Server API for computing average off heap memory consumed
Jackie-Jiang commented on a change in pull request #6172:
URL: https://github.com/apache/incubator-pinot/pull/6172#discussion_r510552367
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/realtime/impl/RealtimeSegmentStatsHistory.java
##########
@@ -341,6 +341,16 @@ public synchronized int getEstimatedRowsToIndex() {
return (numRowsIndexed > 0) ? (int) (numRowsIndexed / numEntriesToScan) : DEFAULT_ROWS_TO_INDEX;
}
+ public synchronized long getLatestSegmentMemoryConsumed() {
+ if (isEmpty()) {
+ return 0;
Review comment:
Return -1 to indicate unavailable?
##########
File path: pinot-core/src/test/java/org/apache/pinot/core/realtime/impl/RealtimeSegmentStatsHistoryTest.java
##########
@@ -250,6 +251,29 @@ public void testVersion1()
Assert.assertEquals(segmentStats.getNumSeconds(), 700);
}
+ @Test
+ public void testLatestConsumedMemory()
+ throws IOException, ClassNotFoundException {
+ final String tmpDir = System.getProperty("java.io.tmpdir");
+ File serializedFile = new File(tmpDir, STATS_FILE_NAME);
+ serializedFile.deleteOnExit();
+ FileUtils.deleteQuietly(serializedFile);
+ long[] memoryValues = {100, 100, 200, 400, 450, 600};
+
+ RealtimeSegmentStatsHistory history = RealtimeSegmentStatsHistory.deserialzeFrom(serializedFile);
+ Assert.assertEquals(history.getLatestSegmentMemoryConsumed(), 0);
+ RealtimeSegmentStatsHistory.SegmentStats segmentStats = null;
+
+ for (int i=0;i< memoryValues.length; i++) {
Review comment:
(nit) reformat
##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/MmapDebugResource.java
##########
@@ -37,6 +48,9 @@
@Path("debug")
public class MmapDebugResource {
+ @Inject
+ ServerInstance serverInstance;
Review comment:
```suggestion
private ServerInstance _serverInstance;
```
##########
File path: pinot-core/src/test/java/org/apache/pinot/core/realtime/impl/RealtimeSegmentStatsHistoryTest.java
##########
@@ -250,6 +251,29 @@ public void testVersion1()
Assert.assertEquals(segmentStats.getNumSeconds(), 700);
}
+ @Test
+ public void testLatestConsumedMemory()
+ throws IOException, ClassNotFoundException {
+ final String tmpDir = System.getProperty("java.io.tmpdir");
+ File serializedFile = new File(tmpDir, STATS_FILE_NAME);
+ serializedFile.deleteOnExit();
+ FileUtils.deleteQuietly(serializedFile);
+ long[] memoryValues = {100, 100, 200, 400, 450, 600};
+
+ RealtimeSegmentStatsHistory history = RealtimeSegmentStatsHistory.deserialzeFrom(serializedFile);
+ Assert.assertEquals(history.getLatestSegmentMemoryConsumed(), 0);
+ RealtimeSegmentStatsHistory.SegmentStats segmentStats = null;
+
+ for (int i=0;i< memoryValues.length; i++) {
+ segmentStats = new RealtimeSegmentStatsHistory.SegmentStats();
+ segmentStats.setMemUsedBytes(memoryValues[i]);
+ history.addSegmentStats(segmentStats);
+ }
+
+ long expectedMemUsed = memoryValues[memoryValues.length-1];
Review comment:
(nit) reformat
##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/MmapDebugResource.java
##########
@@ -45,4 +59,33 @@
public List<String> getOffHeapSizes() {
return PinotDataBuffer.getBufferInfo();
}
+
+ @GET
+ @Produces(MediaType.APPLICATION_JSON)
+ @Path("/tables/{tableName}/memoryConsumedRealtime")
+ @ApiOperation(value = "Show off heap memory consumed by latest mutable segment", notes = "Returns off heap memory consumed by latest consuming segment of realtime table")
+ @ApiResponses(value = {@ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 500, message = "Internal server error"), @ApiResponse(code = 404, message = "Table not found")})
+ public String getTableSize(
+ @ApiParam(value = "Table Name with type", required = true) @PathParam("tableName") String tableName)
+ throws WebApplicationException {
+ double memoryConsumed = 0;
+ TableType tableType = TableNameBuilder.getTableTypeFromTableName(tableName);
+ if (tableType != TableType.REALTIME) {
+ throw new WebApplicationException("This api cannot be used with non real-time table: " + tableName,
+ Response.Status.BAD_REQUEST);
+ }
+
+ InstanceDataManager instanceDataManager = serverInstance.getInstanceDataManager();
+ if (instanceDataManager == null) {
+ throw new WebApplicationException("Invalid server initialization", Response.Status.INTERNAL_SERVER_ERROR);
+ }
+ RealtimeTableDataManager realtimeTableDataManager =
+ (RealtimeTableDataManager) instanceDataManager.getTableDataManager(tableName);
+ if (realtimeTableDataManager == null) {
+ throw new WebApplicationException("Table: " + tableName + " is not found", Response.Status.NOT_FOUND);
+ }
+
+ memoryConsumed = realtimeTableDataManager.getStatsHistory().getLatestSegmentMemoryConsumed();
+ return ResourceUtils.convertToJsonString(memoryConsumed);
Review comment:
This will return a string instead of a json (123 -> "123").
We should create a json to wrap the value.
E.g. `return ResourceUtils.convertToJsonString(Collections.singletonMap("offheapMemoryConsumed", memoryConsumed));`
##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/MmapDebugResource.java
##########
@@ -45,4 +59,33 @@
public List<String> getOffHeapSizes() {
return PinotDataBuffer.getBufferInfo();
}
+
+ @GET
+ @Produces(MediaType.APPLICATION_JSON)
+ @Path("/tables/{tableName}/memoryConsumedRealtime")
+ @ApiOperation(value = "Show off heap memory consumed by latest mutable segment", notes = "Returns off heap memory consumed by latest consuming segment of realtime table")
+ @ApiResponses(value = {@ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 500, message = "Internal server error"), @ApiResponse(code = 404, message = "Table not found")})
+ public String getTableSize(
+ @ApiParam(value = "Table Name with type", required = true) @PathParam("tableName") String tableName)
+ throws WebApplicationException {
+ double memoryConsumed = 0;
+ TableType tableType = TableNameBuilder.getTableTypeFromTableName(tableName);
+ if (tableType != TableType.REALTIME) {
+ throw new WebApplicationException("This api cannot be used with non real-time table: " + tableName,
+ Response.Status.BAD_REQUEST);
+ }
+
+ InstanceDataManager instanceDataManager = serverInstance.getInstanceDataManager();
+ if (instanceDataManager == null) {
+ throw new WebApplicationException("Invalid server initialization", Response.Status.INTERNAL_SERVER_ERROR);
+ }
+ RealtimeTableDataManager realtimeTableDataManager =
+ (RealtimeTableDataManager) instanceDataManager.getTableDataManager(tableName);
+ if (realtimeTableDataManager == null) {
+ throw new WebApplicationException("Table: " + tableName + " is not found", Response.Status.NOT_FOUND);
+ }
+
+ memoryConsumed = realtimeTableDataManager.getStatsHistory().getLatestSegmentMemoryConsumed();
Review comment:
Remove line 71 and declare the variable here. Also, the return value should be `long` instead of `double`
##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/MmapDebugResource.java
##########
@@ -45,4 +59,33 @@
public List<String> getOffHeapSizes() {
return PinotDataBuffer.getBufferInfo();
}
+
+ @GET
+ @Produces(MediaType.APPLICATION_JSON)
+ @Path("/tables/{tableName}/memoryConsumedRealtime")
Review comment:
Recommend `"memory/offheap/table/{tableName}"`
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/realtime/impl/RealtimeSegmentStatsHistory.java
##########
@@ -385,7 +395,7 @@ public static void main(String[] args)
throws Exception {
RealtimeSegmentStatsHistory history = RealtimeSegmentStatsHistory.deserialzeFrom(new File("/tmp/stats.ser"));
System.out.println(history.toString());
- for (int i = 0; i < history.getNumntriesToScan(); i++) {
+ for (int i = 0; i < history.getNumEntriesToScan(); i++) {
Review comment:
Seems like this main method is for some testing during the development, and was accidentally merged. Let's remove it
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org