You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2020/01/03 04:10:47 UTC
[GitHub] [phoenix] nsgupta1 opened a new pull request #670: PHOENIX-5630:
MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED
SQLExceptions should print existing size
nsgupta1 opened a new pull request #670: PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
URL: https://github.com/apache/phoenix/pull/670
Hi @ChinmaySKulkarni , @yanxinyi can you please review this patch ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [phoenix] yanxinyi commented on a change in pull request #670:
PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and
MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #670: PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
URL: https://github.com/apache/phoenix/pull/670#discussion_r362914659
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionInfo.java
##########
@@ -47,6 +51,10 @@
private final String familyName;
private final String columnName;
private final String functionName;
+ private final long maxMutationSize;
+ private final int mutationSize;
Review comment:
nice catch here @mihir6692
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [phoenix] yanxinyi commented on a change in pull request #670:
PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and
MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #670: PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
URL: https://github.com/apache/phoenix/pull/670#discussion_r362953806
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionInfo.java
##########
@@ -47,6 +51,10 @@
private final String familyName;
private final String columnName;
private final String functionName;
+ private final long maxMutationSize;
+ private final int mutationSize;
Review comment:
since we have a `DEFAULT_MAX_MUTATION_SIZE = 500000`, it's ok to use `int` instead of `long` from my point of view. What do you think @ChinmaySKulkarni
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [phoenix] ChinmaySKulkarni closed pull request #670: PHOENIX-5630:
MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED
SQLExceptions should print existing size
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni closed pull request #670: PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
URL: https://github.com/apache/phoenix/pull/670
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request
#670: PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and
MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni commented on a change in pull request #670: PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
URL: https://github.com/apache/phoenix/pull/670#discussion_r363002514
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionInfo.java
##########
@@ -47,6 +51,10 @@
private final String familyName;
private final String columnName;
private final String functionName;
+ private final long maxMutationSize;
+ private final int mutationSize;
Review comment:
@nsgupta1 I think it's fine to keep both to `int` too. But, then for consistency, also better to make the data type change in other relevant places too as @mihir6692 mentioned.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [phoenix] nsgupta1 commented on issue #670: PHOENIX-5630:
MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED
SQLExceptions should print existing size
Posted by GitBox <gi...@apache.org>.
nsgupta1 commented on issue #670: PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
URL: https://github.com/apache/phoenix/pull/670#issuecomment-572329873
Hi @mihir6692 @ChinmaySKulkarni can you please merge the changes and close this PR ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [phoenix] mihir6692 commented on issue #670: PHOENIX-5630:
MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED
SQLExceptions should print existing size
Posted by GitBox <gi...@apache.org>.
mihir6692 commented on issue #670: PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
URL: https://github.com/apache/phoenix/pull/670#issuecomment-571912478
lgtm +1 @nsgupta1
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [phoenix] mihir6692 commented on a change in pull request #670:
PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and
MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
Posted by GitBox <gi...@apache.org>.
mihir6692 commented on a change in pull request #670: PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
URL: https://github.com/apache/phoenix/pull/670#discussion_r362927784
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionInfo.java
##########
@@ -47,6 +51,10 @@
private final String familyName;
private final String columnName;
private final String functionName;
+ private final long maxMutationSize;
+ private final int mutationSize;
Review comment:
I think this is the relevant [code path ](https://github.com/apache/phoenix/blob/356f4cd7d43bccb9538a5a2b94863b1c52cd9aad/phoenix-core/src/main/java/org/apache/phoenix/execute/MutationState.java#L130).
But based on this [QueryServicesOptions](https://github.com/apache/phoenix/blob/78cfb911a157fa74fd9868099e20df3eae6f62ba/phoenix-core/src/main/java/org/apache/phoenix/query/QueryServicesOptions.java#L147) Number of max rows in single batch are limited to int range.
What do you think? @yanxinyi @ChinmaySKulkarni
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [phoenix] mihir6692 commented on a change in pull request #670:
PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and
MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
Posted by GitBox <gi...@apache.org>.
mihir6692 commented on a change in pull request #670: PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
URL: https://github.com/apache/phoenix/pull/670#discussion_r362901682
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionInfo.java
##########
@@ -47,6 +51,10 @@
private final String familyName;
private final String columnName;
private final String functionName;
+ private final long maxMutationSize;
+ private final int mutationSize;
Review comment:
Same for Builder class too
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [phoenix] yanxinyi commented on a change in pull request #670:
PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and
MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #670: PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
URL: https://github.com/apache/phoenix/pull/670#discussion_r362897113
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/schema/MaxMutationSizeBytesExceededException.java
##########
@@ -0,0 +1,31 @@
+package org.apache.phoenix.schema;
Review comment:
please add the Apache license
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [phoenix] yanxinyi commented on issue #670: PHOENIX-5630:
MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED
SQLExceptions should print existing size
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on issue #670: PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
URL: https://github.com/apache/phoenix/pull/670#issuecomment-572324217
LGTM +1
better to create a new JIRA that making the data type changes in another relevant place for consistency.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [phoenix] yanxinyi commented on a change in pull request #670:
PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and
MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #670: PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
URL: https://github.com/apache/phoenix/pull/670#discussion_r362897168
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/schema/MaxMutationSizeExceededException.java
##########
@@ -0,0 +1,28 @@
+package org.apache.phoenix.schema;
Review comment:
same as above, please add the Apache license
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [phoenix] nsgupta1 commented on a change in pull request #670:
PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and
MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
Posted by GitBox <gi...@apache.org>.
nsgupta1 commented on a change in pull request #670: PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
URL: https://github.com/apache/phoenix/pull/670#discussion_r363980746
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionInfo.java
##########
@@ -47,6 +51,10 @@
private final String familyName;
private final String columnName;
private final String functionName;
+ private final long maxMutationSize;
+ private final int mutationSize;
Review comment:
Changes maxSize variable to int from long for consistency.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [phoenix] nsgupta1 edited a comment on issue #670: PHOENIX-5630:
MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED
SQLExceptions should print existing size
Posted by GitBox <gi...@apache.org>.
nsgupta1 edited a comment on issue #670: PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
URL: https://github.com/apache/phoenix/pull/670#issuecomment-572328315
Thanks @mihir6692 and @yanxinyi, I added those data type changes in the same patch DEFAULT_MAX_MUTATION_SIZE is used as an integer only in other classes. It seems consistent to me now.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [phoenix] nsgupta1 commented on a change in pull request #670:
PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and
MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
Posted by GitBox <gi...@apache.org>.
nsgupta1 commented on a change in pull request #670: PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
URL: https://github.com/apache/phoenix/pull/670#discussion_r362921789
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionInfo.java
##########
@@ -47,6 +51,10 @@
private final String familyName;
private final String columnName;
private final String functionName;
+ private final long maxMutationSize;
+ private final int mutationSize;
Review comment:
Thanks @mihir6692 @yanxinyi I had kept it "long" but mutationSize(numRows) in MutationState class is an integer ([here](https://github.com/apache/phoenix/blob/356f4cd7d43bccb9538a5a2b94863b1c52cd9aad/phoenix-core/src/main/java/org/apache/phoenix/execute/MutationState.java#L139)), hence I changed it to int.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [phoenix] mihir6692 commented on a change in pull request #670:
PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and
MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
Posted by GitBox <gi...@apache.org>.
mihir6692 commented on a change in pull request #670: PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
URL: https://github.com/apache/phoenix/pull/670#discussion_r362901319
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionInfo.java
##########
@@ -47,6 +51,10 @@
private final String familyName;
private final String columnName;
private final String functionName;
+ private final long maxMutationSize;
+ private final int mutationSize;
Review comment:
This should be "long" if variable maxMutationSize is also long or vice-a-versa
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [phoenix] nsgupta1 commented on a change in pull request #670:
PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and
MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
Posted by GitBox <gi...@apache.org>.
nsgupta1 commented on a change in pull request #670: PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
URL: https://github.com/apache/phoenix/pull/670#discussion_r363980746
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionInfo.java
##########
@@ -47,6 +51,10 @@
private final String familyName;
private final String columnName;
private final String functionName;
+ private final long maxMutationSize;
+ private final int mutationSize;
Review comment:
Changed maxSize variable to int from long for consistency.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [phoenix] nsgupta1 commented on issue #670: PHOENIX-5630:
MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED
SQLExceptions should print existing size
Posted by GitBox <gi...@apache.org>.
nsgupta1 commented on issue #670: PHOENIX-5630: MAX_MUTATION_SIZE_EXCEEDED and MAX_MUTATION_SIZE_BYTES_EXCEEDED SQLExceptions should print existing size
URL: https://github.com/apache/phoenix/pull/670#issuecomment-572328315
Thanks @mihir6692 and @yanxinyi, I added those data type changes in the same patch DEFAULT_MAX_MUTATION_SIZE is used as an integer only in other places.
----------------------------------------------------------------
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
With regards,
Apache Git Services