Jackie-Jiang commented on a change in pull request #7784:
URL: https://github.com/apache/pinot/pull/7784#discussion_r751776077
##########
File path:
pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/JsonAsyncHttpPinotClientTransport.java
##########
@@ -167,7 +170,7 @@ public boolean isDone() {
@Override
public BrokerResponse get()
throws ExecutionException {
- return get(1000L, TimeUnit.DAYS);
+ return get(60000L * 20L, TimeUnit.NANOSECONDS);
Review comment:
I believe you want to put `MILLISECONDS`?
##########
File path:
pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/Connection.java
##########
@@ -209,7 +222,7 @@ public boolean isDone() {
public ResultSetGroup get()
throws InterruptedException, ExecutionException {
try {
- return get(1000L, TimeUnit.DAYS);
+ return get(200000L, TimeUnit.NANOSECONDS);
Review comment:
Timeout too small? `200000ns = 0.2ms`
##########
File path:
pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/JsonAsyncHttpPinotClientTransport.java
##########
@@ -65,14 +65,17 @@ public JsonAsyncHttpPinotClientTransport(Map<String,
String> headers, String sch
builder.setSSLContext(sslContext);
}
+ builder.setReadTimeout(60000 * 5);
Review comment:
Did you run into problem with the default settings? 5 minutes
connect/read timeout seems too long to me
##########
File path:
pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/PinotStatement.java
##########
@@ -34,7 +34,7 @@
private ResultSetGroup _resultSetGroup;
private boolean _closed;
private ResultSet _resultSet;
- private int _maxRows = Integer.MAX_VALUE;
+ private int _maxRows = 1000000;
Review comment:
Should we validate the query with `LIMIT` and reject queries with limit
larger than this?
##########
File path:
pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/JsonAsyncHttpPinotClientTransport.java
##########
@@ -56,7 +56,7 @@ public JsonAsyncHttpPinotClientTransport() {
}
public JsonAsyncHttpPinotClientTransport(Map<String, String> headers, String
scheme,
- @Nullable SSLContext sslContext) {
+ @Nullable SSLContext sslContext) {
Review comment:
Let's use `Pinot Style`:
https://docs.pinot.apache.org/developers/developers-and-contributors/code-setup#setup-ide
--
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
