[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #11710: Reduce direct memory OOM chances on broker with a per server query response size budget

[via GitHub]

Jackie-Jiang commented on code in PR #11710:
URL: https://github.com/apache/pinot/pull/11710#discussion_r1341914079


##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -362,6 +365,11 @@ public static class QueryOptionKey {
         public static final String MAX_ROWS_IN_JOIN = "maxRowsInJoin";
         public static final String JOIN_OVERFLOW_MODE = "joinOverflowMode";
 
+        // Indicates the maximum length of the serialized response per server 
for a query. The response size that a
+        // server can return is allocated by dividing the total budget 
CONFIG_OF_MAX_QUERY_RESPONSE_SIZE_BYTES across
+        // all servers processing the query.
+        public static final String MAX_SERIALIZED_RESPONSE_LENGTH_PER_SERVER = 
"maxSerializedResponseLengthPerServer";

Review Comment:
   ```suggestion
           public static final String MAX_SERVER_RESPONSE_SIZE_BYTES = 
"maxServerResponseSizeBytes";
   ```



##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -318,6 +318,9 @@ public static class Broker {
         "pinot.broker.enable.partition.metadata.manager";
     public static final boolean DEFAULT_ENABLE_PARTITION_METADATA_MANAGER = 
false;
 
+    public static final String CONFIG_OF_MAX_QUERY_RESPONSE_SIZE_BYTES = 
"pinot.broker.max.query.response.size.bytes";

Review Comment:
   ```suggestion
       public static final String CONFIG_OF_MAX_SERVER_RESPONSE_SIZE_BYTES = 
"pinot.broker.max.server.response.size.bytes";
   ```



##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -318,6 +318,9 @@ public static class Broker {
         "pinot.broker.enable.partition.metadata.manager";
     public static final boolean DEFAULT_ENABLE_PARTITION_METADATA_MANAGER = 
false;
 
+    public static final String CONFIG_OF_MAX_QUERY_RESPONSE_SIZE_BYTES = 
"pinot.broker.max.query.response.size.bytes";
+    public static final long DEFAULT_MAX_QUERY_RESPONSE_SIZE_BYTES = -1;

Review Comment:
   I'd suggest putting a default (e.g. 1GB if not too big)



-- 
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: commits-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org