siddharthteotia commented on PR #11710: URL: https://github.com/apache/pinot/pull/11710#issuecomment-1746378629
I am still not convinced of how we have wired the config aspect. At a high level, I suggest we do the same thing as `timeOut` config since that is exactly what we seem to be doing here with an instance level config and then a query level config (in the form of query option) IIUC. So may be should also leverage queryConfig at the table level as well and have the preference as: `queryOption (query level) > table level > instance level ` Also this may be a nit but the following 2 configs in CommonConstants is not super intuitive to me. `public static final long DEFAULT_MAX_QUERY_RESPONSE_SIZE_BYTES = Long.MAX_VALUE;` `public static final String MAX_SERVER_RESPONSE_SIZE_BYTES = "maxServerResponseSizeBytes";` One problem I see is that while (1) is an instance config but tells for the query and therefore we internally compute the per server threshold / budget during routing depending on how many servers we are routing to. However, (2) seems to be the queryOption way of doing things but here we are having the user directly specify the server level budget instead of overall. Why this difference ? -- 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
