You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "lightning-L (via GitHub)" <gi...@apache.org> on 2023/02/21 13:59:21 UTC

[GitHub] [kyuubi] lightning-L opened a new pull request, #4395: [KYUUBI #4388] Limit the max rows for get nextRowSet api

lightning-L opened a new pull request, #4395:
URL: https://github.com/apache/kyuubi/pull/4395

   <!--
   Thanks for sending a pull request!
   
   Here are some tips for you:
     1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/CONTRIBUTING.html
     2. If the PR is related to an issue in https://github.com/apache/kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
     3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
   -->
   
   ### _Why are the changes needed?_
   <!--
   Please clarify why the changes are needed. For instance,
     1. If you add a feature, you can talk about the use case of it.
     2. If you fix a bug, you can clarify why it is a bug.
   -->
   
   
   ### _How was this patch tested?_
   - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible
   
   - [ ] Add screenshots for manual tests if appropriate
   
   - [ ] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request
   


-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [kyuubi] turboFei commented on a diff in pull request #4395: [KYUUBI #4388] Limit the max rows for get nextRowSet api

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4395:
URL: https://github.com/apache/kyuubi/pull/4395#discussion_r1113804920


##########
docs/deployment/settings.md:
##########
@@ -440,6 +440,7 @@ You can configure the Kyuubi properties in `$KYUUBI_HOME/conf/kyuubi-defaults.co
 | kyuubi.operation.plan.only.output.style         | plain                                                                           | Configures the planOnly output style. The value can be 'plain' or 'json', and the default value is 'plain'. This configuration supports only the output styles of the Spark engine                                                                                                                                                                                                                                                                                                                       | string   | 1.7.0 |
 | kyuubi.operation.progress.enabled               | false                                                                           | Whether to enable the operation progress. When true, the operation progress will be returned in `GetOperationStatus`.                                                                                                                                                                                                                                                                                                                                                                                    | boolean  | 1.6.0 |
 | kyuubi.operation.query.timeout                  | &lt;undefined&gt;                                                               | Timeout for query executions at server-side, take effect with client-side timeout(`java.sql.Statement.setQueryTimeout`) together, a running query will be cancelled automatically if timeout. It's off by default, which means only client-side take full control of whether the query should timeout or not. If set, client-side timeout is capped at this point. To cancel the queries right away without waiting for task to finish, consider enabling kyuubi.operation.interrupt.on.cancel together. | duration | 1.2.0 |
+| kyuubi.operation.rest.fetch.max.rows            | 5000                                                                            | Max rows limit for getting result row set api. If the max rows specified by client-side is larger than the limit, request will fail directly.                                                                                                                                                                                                                                                                                                                                                            | int      | 1.7.0 |

Review Comment:
   not only for rest call, we need also limit the jdbc



-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [kyuubi] turboFei commented on a diff in pull request #4395: [KYUUBI #4388] Limit the max rows for get nextRowSet api

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4395:
URL: https://github.com/apache/kyuubi/pull/4395#discussion_r1113929138


##########
docs/deployment/settings.md:
##########
@@ -431,6 +431,7 @@ You can configure the Kyuubi properties in `$KYUUBI_HOME/conf/kyuubi-defaults.co
 
 |                       Key                       |                                     Default                                     |                                                                                                                                                                                                                                                 Meaning                                                                                                                                                                                                                                                  |   Type   | Since |
 |-------------------------------------------------|---------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|-------|
+| kyuubi.operation.client.fetch.max.rows          | 5000                                                                            | Max rows limit for getting result row set operation. If the max rows specified by client-side is larger than the limit, request will fail directly.                                                                                                                                                                                                                                                                                                                                                      | int      | 1.7.0 |

Review Comment:
   Refer the style liks:
   ```
   kyuubi.server.limit.connections.per.user
   ``
   
   
   how about:
   
   ```
   kyuubi.server.limit.client.fetch.max.rows
   ```



-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [kyuubi] turboFei commented on a diff in pull request #4395: [KYUUBI #4388] Limit the max rows for get nextRowSet api

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4395:
URL: https://github.com/apache/kyuubi/pull/4395#discussion_r1114051644


##########
kyuubi-common/src/main/scala/org/apache/kyuubi/service/AbstractBackendService.scala:
##########
@@ -201,6 +201,11 @@ abstract class AbstractBackendService(name: String)
       orientation: FetchOrientation,
       maxRows: Int,
       fetchLog: Boolean): TRowSet = {
+    val maxRowsLimit = conf.get(KyuubiConf.SERVER_LIMIT_CLIENT_FETCH_MAX_ROWS)

Review Comment:
   how about using a lazy val in the class.
   
   ```
   private lazy val maxRowsLimit = conf.get(KyuubiConf.SERVER_LIMIT_CLIENT_FETCH_MAX_ROWS)
   ```



-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [kyuubi] turboFei commented on pull request #4395: [KYUUBI #4388] Limit the max rows for get nextRowSet api

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #4395:
URL: https://github.com/apache/kyuubi/pull/4395#issuecomment-1439796715

   Af think twice， I think that we can set the default value to optional， to do not break the original behavior.


-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [kyuubi] turboFei commented on a diff in pull request #4395: [KYUUBI #4388] Limit the max rows for get nextRowSet api

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4395:
URL: https://github.com/apache/kyuubi/pull/4395#discussion_r1113929218


##########
docs/deployment/settings.md:
##########
@@ -431,6 +431,7 @@ You can configure the Kyuubi properties in `$KYUUBI_HOME/conf/kyuubi-defaults.co
 
 |                       Key                       |                                     Default                                     |                                                                                                                                                                                                                                                 Meaning                                                                                                                                                                                                                                                  |   Type   | Since |
 |-------------------------------------------------|---------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|-------|
+| kyuubi.operation.client.fetch.max.rows          | 5000                                                                            | Max rows limit for getting result row set operation. If the max rows specified by client-side is larger than the limit, request will fail directly.                                                                                                                                                                                                                                                                                                                                                      | int      | 1.7.0 |

Review Comment:
   
   how about:
   
   ```
   kyuubi.server.limit.maxFetchSize
   ```



##########
docs/deployment/settings.md:
##########
@@ -431,6 +431,7 @@ You can configure the Kyuubi properties in `$KYUUBI_HOME/conf/kyuubi-defaults.co
 
 |                       Key                       |                                     Default                                     |                                                                                                                                                                                                                                                 Meaning                                                                                                                                                                                                                                                  |   Type   | Since |
 |-------------------------------------------------|---------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|-------|
+| kyuubi.operation.client.fetch.max.rows          | 5000                                                                            | Max rows limit for getting result row set operation. If the max rows specified by client-side is larger than the limit, request will fail directly.                                                                                                                                                                                                                                                                                                                                                      | int      | 1.7.0 |

Review Comment:
   
   how about:
   
   ```
   kyuubi.server.limit.max.fetch.size
   ```



-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [kyuubi] turboFei commented on a diff in pull request #4395: [KYUUBI #4388] Limit the max rows for get nextRowSet api

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4395:
URL: https://github.com/apache/kyuubi/pull/4395#discussion_r1113929218


##########
docs/deployment/settings.md:
##########
@@ -431,6 +431,7 @@ You can configure the Kyuubi properties in `$KYUUBI_HOME/conf/kyuubi-defaults.co
 
 |                       Key                       |                                     Default                                     |                                                                                                                                                                                                                                                 Meaning                                                                                                                                                                                                                                                  |   Type   | Since |
 |-------------------------------------------------|---------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|-------|
+| kyuubi.operation.client.fetch.max.rows          | 5000                                                                            | Max rows limit for getting result row set operation. If the max rows specified by client-side is larger than the limit, request will fail directly.                                                                                                                                                                                                                                                                                                                                                      | int      | 1.7.0 |

Review Comment:
   Refer the style liks:
   ```
   kyuubi.server.limit.connections.per.user
   ``
   
   
   how about:
   
   ```
   kyuubi.server.limit.client.fetch.max.rows
   ```



-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [kyuubi] turboFei commented on a diff in pull request #4395: [KYUUBI #4388] Limit the max rows for get nextRowSet api

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4395:
URL: https://github.com/apache/kyuubi/pull/4395#discussion_r1113930064


##########
docs/deployment/settings.md:
##########
@@ -431,6 +431,7 @@ You can configure the Kyuubi properties in `$KYUUBI_HOME/conf/kyuubi-defaults.co
 
 |                       Key                       |                                     Default                                     |                                                                                                                                                                                                                                                 Meaning                                                                                                                                                                                                                                                  |   Type   | Since |
 |-------------------------------------------------|---------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|-------|
+| kyuubi.operation.client.fetch.max.rows          | 5000                                                                            | Max rows limit for getting result row set operation. If the max rows specified by client-side is larger than the limit, request will fail directly.                                                                                                                                                                                                                                                                                                                                                      | int      | 1.7.0 |

Review Comment:
   refer the config:
   
     val SERVER_LIMIT_CONNECTIONS_PER_USER_IPADDRESS: OptionalConfigEntry[Int] =
       buildConf("kyuubi.server.limit.connections.per.user.ipaddress")
         .doc("Maximum kyuubi server connections per user:ipaddress combination." +
           " Any user-ipaddress exceeding this limit will not be allowed to connect.")
         .version("1.6.0")
         .serverOnly
         .intConf
         .createOptional



-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [kyuubi] turboFei commented on a diff in pull request #4395: [KYUUBI #4388] Limit the max rows for get nextRowSet api

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4395:
URL: https://github.com/apache/kyuubi/pull/4395#discussion_r1113929218


##########
docs/deployment/settings.md:
##########
@@ -431,6 +431,7 @@ You can configure the Kyuubi properties in `$KYUUBI_HOME/conf/kyuubi-defaults.co
 
 |                       Key                       |                                     Default                                     |                                                                                                                                                                                                                                                 Meaning                                                                                                                                                                                                                                                  |   Type   | Since |
 |-------------------------------------------------|---------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|-------|
+| kyuubi.operation.client.fetch.max.rows          | 5000                                                                            | Max rows limit for getting result row set operation. If the max rows specified by client-side is larger than the limit, request will fail directly.                                                                                                                                                                                                                                                                                                                                                      | int      | 1.7.0 |

Review Comment:
   
   how about:
   
   ```
   kyuubi.server.limit.client.fetch.max.rows
   ```



-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [kyuubi] turboFei commented on a diff in pull request #4395: [KYUUBI #4388] Limit the max rows for get nextRowSet api

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4395:
URL: https://github.com/apache/kyuubi/pull/4395#discussion_r1114036010


##########
docs/deployment/settings.md:
##########
@@ -455,6 +455,7 @@ You can configure the Kyuubi properties in `$KYUUBI_HOME/conf/kyuubi-defaults.co
 | kyuubi.server.batch.limit.connections.per.user           | &lt;undefined&gt; | Maximum kyuubi server batch connections per user. Any user exceeding this limit will not be allowed to connect.                                                                                                                                | int      | 1.7.0 |
 | kyuubi.server.batch.limit.connections.per.user.ipaddress | &lt;undefined&gt; | Maximum kyuubi server batch connections per user:ipaddress combination. Any user-ipaddress exceeding this limit will not be allowed to connect.                                                                                                | int      | 1.7.0 |
 | kyuubi.server.info.provider                              | ENGINE            | The server information provider name, some clients may rely on this information to check the server compatibilities and functionalities. <li>SERVER: Return Kyuubi server information.</li> <li>ENGINE: Return Kyuubi engine information.</li> | string   | 1.6.1 |
+| kyuubi.server.limit.client.fetch.max.rows                | 5000              | Max rows limit for getting result row set operation. If the max rows specified by client-side is larger than the limit, request will fail directly.                                                                                            | int      | 1.7.0 |

Review Comment:
   raised #4398



-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [kyuubi] turboFei commented on pull request #4395: [KYUUBI #4388] Limit the max rows for get nextRowSet api

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on PR #4395:
URL: https://github.com/apache/kyuubi/pull/4395#issuecomment-1441101839

   thanks, merging to master


-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [kyuubi] turboFei commented on a diff in pull request #4395: [KYUUBI #4388] Limit the max rows for get nextRowSet api

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4395:
URL: https://github.com/apache/kyuubi/pull/4395#discussion_r1114051644


##########
kyuubi-common/src/main/scala/org/apache/kyuubi/service/AbstractBackendService.scala:
##########
@@ -201,6 +201,11 @@ abstract class AbstractBackendService(name: String)
       orientation: FetchOrientation,
       maxRows: Int,
       fetchLog: Boolean): TRowSet = {
+    val maxRowsLimit = conf.get(KyuubiConf.SERVER_LIMIT_CLIENT_FETCH_MAX_ROWS)

Review Comment:
   how about using a lazy val.
   
   ```
   private lazy val maxRowsLimit = conf.get(KyuubiConf.SERVER_LIMIT_CLIENT_FETCH_MAX_ROWS)
   ```



-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [kyuubi] turboFei commented on a diff in pull request #4395: [KYUUBI #4388] Limit the max rows for get nextRowSet api

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4395:
URL: https://github.com/apache/kyuubi/pull/4395#discussion_r1113929218


##########
docs/deployment/settings.md:
##########
@@ -431,6 +431,7 @@ You can configure the Kyuubi properties in `$KYUUBI_HOME/conf/kyuubi-defaults.co
 
 |                       Key                       |                                     Default                                     |                                                                                                                                                                                                                                                 Meaning                                                                                                                                                                                                                                                  |   Type   | Since |
 |-------------------------------------------------|---------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|-------|
+| kyuubi.operation.client.fetch.max.rows          | 5000                                                                            | Max rows limit for getting result row set operation. If the max rows specified by client-side is larger than the limit, request will fail directly.                                                                                                                                                                                                                                                                                                                                                      | int      | 1.7.0 |

Review Comment:
   
   how about:
   
   ```
   kyuubi.server.limit.max.fetch.size
   ```
   
   cc @pan3793 



-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [kyuubi] lightning-L commented on pull request #4395: [KYUUBI #4388] Limit the max rows for get nextRowSet api

Posted by "lightning-L (via GitHub)" <gi...@apache.org>.
lightning-L commented on PR #4395:
URL: https://github.com/apache/kyuubi/pull/4395#issuecomment-1439353639

   @turboFei 


-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [kyuubi] turboFei closed pull request #4395: [KYUUBI #4388] Limit the max rows for get nextRowSet api

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei closed pull request #4395: [KYUUBI #4388] Limit the max rows for get nextRowSet api
URL: https://github.com/apache/kyuubi/pull/4395


-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [kyuubi] codecov-commenter commented on pull request #4395: [KYUUBI #4388] Limit the max rows for get nextRowSet api

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #4395:
URL: https://github.com/apache/kyuubi/pull/4395#issuecomment-1439616501

   # [Codecov](https://codecov.io/gh/apache/kyuubi/pull/4395?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#4395](https://codecov.io/gh/apache/kyuubi/pull/4395?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3feabf7) into [master](https://codecov.io/gh/apache/kyuubi/commit/3b73e1d64af8cb6351936d277ff57d98faa30c41?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3b73e1d) will **decrease** coverage by `0.04%`.
   > The diff coverage is `88.88%`.
   
   > :exclamation: Current head 3feabf7 differs from pull request most recent head 87c7e26. Consider uploading reports for the commit 87c7e26 to get more accurate results
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #4395      +/-   ##
   ============================================
   - Coverage     53.74%   53.71%   -0.04%     
     Complexity       13       13              
   ============================================
     Files           564      564              
     Lines         30895    30904       +9     
     Branches       4164     4166       +2     
   ============================================
   - Hits          16605    16600       -5     
   - Misses        12736    12749      +13     
   - Partials       1554     1555       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/kyuubi/pull/4395?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://codecov.io/gh/apache/kyuubi/pull/4395?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | `97.40% <83.33%> (-0.06%)` | :arrow_down: |
   | [...ache/kyuubi/server/api/v1/OperationsResource.scala](https://codecov.io/gh/apache/kyuubi/pull/4395?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvYXBpL3YxL09wZXJhdGlvbnNSZXNvdXJjZS5zY2FsYQ==) | `73.26% <100.00%> (+0.81%)` | :arrow_up: |
   | [.../kyuubi/server/mysql/constant/MySQLErrorCode.scala](https://codecov.io/gh/apache/kyuubi/pull/4395?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbXlzcWwvY29uc3RhbnQvTXlTUUxFcnJvckNvZGUuc2NhbGE=) | `13.84% <0.00%> (-6.16%)` | :arrow_down: |
   | [...ache/kyuubi/server/mysql/MySQLCommandHandler.scala](https://codecov.io/gh/apache/kyuubi/pull/4395?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbXlzcWwvTXlTUUxDb21tYW5kSGFuZGxlci5zY2FsYQ==) | `77.77% <0.00%> (-4.05%)` | :arrow_down: |
   | [...g/apache/kyuubi/operation/BatchJobSubmission.scala](https://codecov.io/gh/apache/kyuubi/pull/4395?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vQmF0Y2hKb2JTdWJtaXNzaW9uLnNjYWxh) | `75.27% <0.00%> (-2.20%)` | :arrow_down: |
   | [...ache/kyuubi/server/mysql/MySQLGenericPackets.scala](https://codecov.io/gh/apache/kyuubi/pull/4395?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbXlzcWwvTXlTUUxHZW5lcmljUGFja2V0cy5zY2FsYQ==) | `76.59% <0.00%> (-2.13%)` | :arrow_down: |
   | [...g/apache/kyuubi/session/KyuubiSessionManager.scala](https://codecov.io/gh/apache/kyuubi/pull/4395?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL0t5dXViaVNlc3Npb25NYW5hZ2VyLnNjYWxh) | `94.36% <0.00%> (-0.71%)` | :arrow_down: |
   | [...rg/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala](https://codecov.io/gh/apache/kyuubi/pull/4395?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvY21kL2xvZy9Mb2dCYXRjaENvbW1hbmQuc2NhbGE=) | `60.60% <0.00%> (+1.51%)` | :arrow_up: |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [kyuubi] turboFei commented on a diff in pull request #4395: [KYUUBI #4388] Limit the max rows for get nextRowSet api

Posted by "turboFei (via GitHub)" <gi...@apache.org>.
turboFei commented on code in PR #4395:
URL: https://github.com/apache/kyuubi/pull/4395#discussion_r1114029845


##########
docs/deployment/settings.md:
##########
@@ -455,6 +455,7 @@ You can configure the Kyuubi properties in `$KYUUBI_HOME/conf/kyuubi-defaults.co
 | kyuubi.server.batch.limit.connections.per.user           | &lt;undefined&gt; | Maximum kyuubi server batch connections per user. Any user exceeding this limit will not be allowed to connect.                                                                                                                                | int      | 1.7.0 |
 | kyuubi.server.batch.limit.connections.per.user.ipaddress | &lt;undefined&gt; | Maximum kyuubi server batch connections per user:ipaddress combination. Any user-ipaddress exceeding this limit will not be allowed to connect.                                                                                                | int      | 1.7.0 |
 | kyuubi.server.info.provider                              | ENGINE            | The server information provider name, some clients may rely on this information to check the server compatibilities and functionalities. <li>SERVER: Return Kyuubi server information.</li> <li>ENGINE: Return Kyuubi engine information.</li> | string   | 1.6.1 |
+| kyuubi.server.limit.client.fetch.max.rows                | 5000              | Max rows limit for getting result row set operation. If the max rows specified by client-side is larger than the limit, request will fail directly.                                                                                            | int      | 1.7.0 |

Review Comment:
   ```
   kyuubi.server.limit.connections.per.user.ipaddress
   kyuubi.server.batch.limit.connections.per.user.ipaddress
   kyuubi.server.limit.client.fetch.max.rows   
   ```
   
   
   should we rename the config `kyuubi.server.batch.limit.connections.per.user.ipaddress` to `kyuubi.server.limit.batch.connections.per.user.ipaddress` ? @pan3793 



-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org