You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by "haridsv (via GitHub)" <gi...@apache.org> on 2023/03/02 15:59:41 UTC
[GitHub] [phoenix] haridsv opened a new pull request, #1570: PHOENIX-6821: Optimize batching in auto-commit mode
haridsv opened a new pull request, #1570:
URL: https://github.com/apache/phoenix/pull/1570
This change also includes the following to make the batch functionality more consistent and confirmant to the JDBC spec:
- Disallow the use of DQL in a batch by throwing BatchUpdateException.
- When addBatch is called if we already have a non-empty MutationState, then treat it as an error, i.e., prohibit mixing batch and non-batch executions.
- Replace the use of custom BatchUpdateExecution exception with the java.sql.BatchUpdateException and propagate update counts through it.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] haridsv commented on a diff in pull request #1570: PHOENIX-6821: Optimize batching in auto-commit mode
Posted by "haridsv (via GitHub)" <gi...@apache.org>.
haridsv commented on code in PR #1570:
URL: https://github.com/apache/phoenix/pull/1570#discussion_r1133873408
##########
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java:
##########
@@ -1993,19 +1992,31 @@ public void clearBatch() throws SQLException {
@Override
public int[] executeBatch() throws SQLException {
int i = 0;
+ int[] returnCodes = new int [batch.size()];
+ Arrays.fill(returnCodes, -1);
+ boolean autoCommit = connection.getAutoCommit();
+ connection.setAutoCommit(false);
try {
- int[] returnCodes = new int [batch.size()];
for (i = 0; i < returnCodes.length; i++) {
PhoenixPreparedStatement statement = batch.get(i);
- returnCodes[i] = statement.execute(true) ? Statement.SUCCESS_NO_INFO : statement.getUpdateCount();
+ statement.executeForBatch();
+ returnCodes[i] = statement.getUpdateCount();
}
// Flush all changes in batch if auto flush is true
flushIfNecessary();
// If we make it all the way through, clear the batch
clearBatch();
return returnCodes;
- } catch (Throwable t) {
- throw new BatchUpdateExecution(t,i);
+ } catch (SQLException t) {
+ throw new BatchUpdateException(returnCodes, t);
Review Comment:
First of all, please note that I am actually initializing the array with `-1` which is currently an invalid value. I know that it is not formally reserved as an invalid value, but JDBC specifically leaves this out and uses `-2` for `SUCCESS_NO_INFO` and `-3` for `EXECUTE_FAILED` so it seems like they had this in their mind and so seems to be safe.
Regarding your thought, at this location, we would know which statement failed, so we could very well set that index alone to `EXECUTE_FAILED`, however, if the exception occurs in commit, you are right that we won't know. However, this is going to be the case even when `autocommit` is `false` so I think there is benefit in keeping the behavior same here as well.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] haridsv commented on a diff in pull request #1570: PHOENIX-6821: Optimize batching in auto-commit mode
Posted by "haridsv (via GitHub)" <gi...@apache.org>.
haridsv commented on code in PR #1570:
URL: https://github.com/apache/phoenix/pull/1570#discussion_r1138902532
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/UpsertValuesIT.java:
##########
@@ -480,10 +488,14 @@ public void testBatchedUpsert() throws Exception {
pstmt.addBatch();
pstmt.executeBatch();
conn.commit();
Review Comment:
I agree with your philosophy on this, I have done this as well.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] haridsv commented on a diff in pull request #1570: PHOENIX-6821: Optimize batching in auto-commit mode
Posted by "haridsv (via GitHub)" <gi...@apache.org>.
haridsv commented on code in PR #1570:
URL: https://github.com/apache/phoenix/pull/1570#discussion_r1135037291
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/UpsertValuesIT.java:
##########
@@ -480,10 +488,14 @@ public void testBatchedUpsert() throws Exception {
pstmt.addBatch();
pstmt.executeBatch();
conn.commit();
Review Comment:
Isn't it harmless to call commit (a 2nd time) even `if (autocommit)`?
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] haridsv commented on a diff in pull request #1570: PHOENIX-6821: Optimize batching in auto-commit mode
Posted by "haridsv (via GitHub)" <gi...@apache.org>.
haridsv commented on code in PR #1570:
URL: https://github.com/apache/phoenix/pull/1570#discussion_r1135073944
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/UpsertValuesIT.java:
##########
@@ -480,10 +488,14 @@ public void testBatchedUpsert() throws Exception {
pstmt.addBatch();
pstmt.executeBatch();
conn.commit();
+ PhoenixConnection pConn = conn.unwrap(PhoenixConnection.class);
+ Map<String, Map<MetricType, Long>> mutationMetrics = pConn.getMutationMetrics();
Review Comment:
Actually, `updateCount` in `MutationState` gets reset when `connection.commit()` is called and since it gets called implicitly as part of `Statement.executeBatch()`, it will always be `0` if we check from this test. Do you have any idea on the appropriate location?
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] haridsv commented on a diff in pull request #1570: PHOENIX-6821: Optimize batching in auto-commit mode
Posted by "haridsv (via GitHub)" <gi...@apache.org>.
haridsv commented on code in PR #1570:
URL: https://github.com/apache/phoenix/pull/1570#discussion_r1133815845
##########
pom.xml:
##########
@@ -98,7 +98,7 @@
<jackson-bom.version>2.12.6.20220326</jackson-bom.version>
<antlr.version>3.5.2</antlr.version>
<!-- Only used for tests with HBase 2.1-2.4 -->
- <reload4j.version>1.2.19</reload4j.version>
+ <reload4j.version>1.2.21</reload4j.version>
Review Comment:
Yes, it is not, but I was getting build error initially which got fixed with this change. I will try again without this change.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] tkhurana commented on pull request #1570: PHOENIX-6821: Optimize batching in auto-commit mode
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on PR #1570:
URL: https://github.com/apache/phoenix/pull/1570#issuecomment-1495109837
@haridsv There are still some checkstyle warnings left.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] tkhurana commented on a diff in pull request #1570: PHOENIX-6821: Optimize batching in auto-commit mode
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #1570:
URL: https://github.com/apache/phoenix/pull/1570#discussion_r1131685158
##########
pom.xml:
##########
@@ -98,7 +98,7 @@
<jackson-bom.version>2.12.6.20220326</jackson-bom.version>
<antlr.version>3.5.2</antlr.version>
<!-- Only used for tests with HBase 2.1-2.4 -->
- <reload4j.version>1.2.19</reload4j.version>
+ <reload4j.version>1.2.21</reload4j.version>
Review Comment:
This doesn't seem related to this PR
##########
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java:
##########
@@ -1993,19 +1992,31 @@ public void clearBatch() throws SQLException {
@Override
public int[] executeBatch() throws SQLException {
int i = 0;
+ int[] returnCodes = new int [batch.size()];
+ Arrays.fill(returnCodes, -1);
+ boolean autoCommit = connection.getAutoCommit();
+ connection.setAutoCommit(false);
Review Comment:
we need to restore the state of auto commit setting before we return from this function
##########
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java:
##########
@@ -1993,19 +1992,31 @@ public void clearBatch() throws SQLException {
@Override
public int[] executeBatch() throws SQLException {
int i = 0;
+ int[] returnCodes = new int [batch.size()];
+ Arrays.fill(returnCodes, -1);
+ boolean autoCommit = connection.getAutoCommit();
+ connection.setAutoCommit(false);
try {
- int[] returnCodes = new int [batch.size()];
for (i = 0; i < returnCodes.length; i++) {
PhoenixPreparedStatement statement = batch.get(i);
- returnCodes[i] = statement.execute(true) ? Statement.SUCCESS_NO_INFO : statement.getUpdateCount();
+ statement.executeForBatch();
+ returnCodes[i] = statement.getUpdateCount();
}
// Flush all changes in batch if auto flush is true
flushIfNecessary();
// If we make it all the way through, clear the batch
clearBatch();
return returnCodes;
- } catch (Throwable t) {
- throw new BatchUpdateExecution(t,i);
+ } catch (SQLException t) {
+ throw new BatchUpdateException(returnCodes, t);
+ } finally {
+ if (autoCommit) {
+ try {
+ connection.commit();
Review Comment:
Because you are calling commit in finally block the `clearBatch()` call has already happened so if the commit throws an exception we would have modified the state of the batch.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] haridsv commented on a diff in pull request #1570: PHOENIX-6821: Optimize batching in auto-commit mode
Posted by "haridsv (via GitHub)" <gi...@apache.org>.
haridsv commented on code in PR #1570:
URL: https://github.com/apache/phoenix/pull/1570#discussion_r1133831002
##########
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java:
##########
@@ -1993,19 +1992,31 @@ public void clearBatch() throws SQLException {
@Override
public int[] executeBatch() throws SQLException {
int i = 0;
+ int[] returnCodes = new int [batch.size()];
+ Arrays.fill(returnCodes, -1);
+ boolean autoCommit = connection.getAutoCommit();
+ connection.setAutoCommit(false);
try {
- int[] returnCodes = new int [batch.size()];
for (i = 0; i < returnCodes.length; i++) {
PhoenixPreparedStatement statement = batch.get(i);
- returnCodes[i] = statement.execute(true) ? Statement.SUCCESS_NO_INFO : statement.getUpdateCount();
+ statement.executeForBatch();
+ returnCodes[i] = statement.getUpdateCount();
}
// Flush all changes in batch if auto flush is true
flushIfNecessary();
// If we make it all the way through, clear the batch
clearBatch();
return returnCodes;
- } catch (Throwable t) {
- throw new BatchUpdateExecution(t,i);
+ } catch (SQLException t) {
+ throw new BatchUpdateException(returnCodes, t);
+ } finally {
+ if (autoCommit) {
+ try {
+ connection.commit();
Review Comment:
In the try block reached the end, then the batch state has already been transferred to the MutationState, so isn't clearing the batch the right approach?
On the other hand, I see issues with calling commit, even when there is an exception being already on the stack. I will fix that.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] tkhurana commented on a diff in pull request #1570: PHOENIX-6821: Optimize batching in auto-commit mode
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #1570:
URL: https://github.com/apache/phoenix/pull/1570#discussion_r1131720519
##########
phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionCode.java:
##########
@@ -649,6 +652,14 @@ public SQLException newException(SQLExceptionInfo info) {
return new PhoenixParserException(info.getMessage(), info.getRootCause());
}
+ };
+ public static final Factory BATCH = new Factory() {
Review Comment:
Maybe use name BATCH_UPDATE_ERROR
##########
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java:
##########
@@ -1993,19 +1992,31 @@ public void clearBatch() throws SQLException {
@Override
public int[] executeBatch() throws SQLException {
int i = 0;
+ int[] returnCodes = new int [batch.size()];
+ Arrays.fill(returnCodes, -1);
+ boolean autoCommit = connection.getAutoCommit();
+ connection.setAutoCommit(false);
try {
- int[] returnCodes = new int [batch.size()];
for (i = 0; i < returnCodes.length; i++) {
PhoenixPreparedStatement statement = batch.get(i);
- returnCodes[i] = statement.execute(true) ? Statement.SUCCESS_NO_INFO : statement.getUpdateCount();
+ statement.executeForBatch();
+ returnCodes[i] = statement.getUpdateCount();
}
// Flush all changes in batch if auto flush is true
flushIfNecessary();
// If we make it all the way through, clear the batch
clearBatch();
return returnCodes;
- } catch (Throwable t) {
- throw new BatchUpdateExecution(t,i);
+ } catch (SQLException t) {
+ throw new BatchUpdateException(returnCodes, t);
Review Comment:
On exceptions shouldn't we set all the return codes to `Statement.EXECUTE_FAILED` since we don't know which individual query failed ?
##########
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java:
##########
@@ -1993,19 +1992,31 @@ public void clearBatch() throws SQLException {
@Override
public int[] executeBatch() throws SQLException {
int i = 0;
+ int[] returnCodes = new int [batch.size()];
Review Comment:
The javadoc for `executeBatch()` function needs to be updated
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] haridsv commented on a diff in pull request #1570: PHOENIX-6821: Optimize batching in auto-commit mode
Posted by "haridsv (via GitHub)" <gi...@apache.org>.
haridsv commented on code in PR #1570:
URL: https://github.com/apache/phoenix/pull/1570#discussion_r1138899925
##########
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java:
##########
@@ -1993,19 +1992,31 @@ public void clearBatch() throws SQLException {
@Override
public int[] executeBatch() throws SQLException {
int i = 0;
+ int[] returnCodes = new int [batch.size()];
+ Arrays.fill(returnCodes, -1);
+ boolean autoCommit = connection.getAutoCommit();
+ connection.setAutoCommit(false);
try {
- int[] returnCodes = new int [batch.size()];
for (i = 0; i < returnCodes.length; i++) {
PhoenixPreparedStatement statement = batch.get(i);
- returnCodes[i] = statement.execute(true) ? Statement.SUCCESS_NO_INFO : statement.getUpdateCount();
+ statement.executeForBatch();
+ returnCodes[i] = statement.getUpdateCount();
}
// Flush all changes in batch if auto flush is true
flushIfNecessary();
// If we make it all the way through, clear the batch
clearBatch();
return returnCodes;
- } catch (Throwable t) {
- throw new BatchUpdateExecution(t,i);
+ } catch (SQLException t) {
+ throw new BatchUpdateException(returnCodes, t);
+ } finally {
+ if (autoCommit) {
+ try {
+ connection.commit();
Review Comment:
I have now moved the commit() inside the try block and removed the propagation of `returnCodes`.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] tkhurana commented on a diff in pull request #1570: PHOENIX-6821: Optimize batching in auto-commit mode
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #1570:
URL: https://github.com/apache/phoenix/pull/1570#discussion_r1131742664
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/UpsertValuesIT.java:
##########
@@ -480,10 +488,14 @@ public void testBatchedUpsert() throws Exception {
pstmt.addBatch();
pstmt.executeBatch();
conn.commit();
+ PhoenixConnection pConn = conn.unwrap(PhoenixConnection.class);
+ Map<String, Map<MetricType, Long>> mutationMetrics = pConn.getMutationMetrics();
Review Comment:
You should also check that the underlying mutation state inside connection has joined all the statements in the batch. You can check the value of `connection.getMutationState().getUpdateCount()` The above check doesn't really tell me if the DMLs executed individually or together. The mutationstate check will confirm that all the DMLs were sent in a batch.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] haridsv commented on a diff in pull request #1570: PHOENIX-6821: Optimize batching in auto-commit mode
Posted by "haridsv (via GitHub)" <gi...@apache.org>.
haridsv commented on code in PR #1570:
URL: https://github.com/apache/phoenix/pull/1570#discussion_r1133879270
##########
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java:
##########
@@ -1993,19 +1992,31 @@ public void clearBatch() throws SQLException {
@Override
public int[] executeBatch() throws SQLException {
int i = 0;
+ int[] returnCodes = new int [batch.size()];
+ Arrays.fill(returnCodes, -1);
+ boolean autoCommit = connection.getAutoCommit();
+ connection.setAutoCommit(false);
Review Comment:
Good catch! I saved the value but forgot to use it.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] haridsv commented on a diff in pull request #1570: PHOENIX-6821: Optimize batching in auto-commit mode
Posted by "haridsv (via GitHub)" <gi...@apache.org>.
haridsv commented on code in PR #1570:
URL: https://github.com/apache/phoenix/pull/1570#discussion_r1138901201
##########
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java:
##########
@@ -1993,19 +1992,31 @@ public void clearBatch() throws SQLException {
@Override
public int[] executeBatch() throws SQLException {
int i = 0;
+ int[] returnCodes = new int [batch.size()];
+ Arrays.fill(returnCodes, -1);
+ boolean autoCommit = connection.getAutoCommit();
+ connection.setAutoCommit(false);
try {
- int[] returnCodes = new int [batch.size()];
for (i = 0; i < returnCodes.length; i++) {
PhoenixPreparedStatement statement = batch.get(i);
- returnCodes[i] = statement.execute(true) ? Statement.SUCCESS_NO_INFO : statement.getUpdateCount();
+ statement.executeForBatch();
+ returnCodes[i] = statement.getUpdateCount();
}
// Flush all changes in batch if auto flush is true
flushIfNecessary();
// If we make it all the way through, clear the batch
clearBatch();
return returnCodes;
- } catch (Throwable t) {
- throw new BatchUpdateExecution(t,i);
+ } catch (SQLException t) {
+ throw new BatchUpdateException(returnCodes, t);
Review Comment:
As mentioned in another thread, now `returnCodes` will exist only when for loop fails.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] tkhurana commented on a diff in pull request #1570: PHOENIX-6821: Optimize batching in auto-commit mode
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #1570:
URL: https://github.com/apache/phoenix/pull/1570#discussion_r1131742664
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/UpsertValuesIT.java:
##########
@@ -480,10 +488,14 @@ public void testBatchedUpsert() throws Exception {
pstmt.addBatch();
pstmt.executeBatch();
conn.commit();
+ PhoenixConnection pConn = conn.unwrap(PhoenixConnection.class);
+ Map<String, Map<MetricType, Long>> mutationMetrics = pConn.getMutationMetrics();
Review Comment:
You should also check that the underlying mutation state inside connection has joined all the statements in the batch. You can check the value of `connection.getMutationState().getUpdateCount()`
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] haridsv commented on a diff in pull request #1570: PHOENIX-6821: Optimize batching in auto-commit mode
Posted by "haridsv (via GitHub)" <gi...@apache.org>.
haridsv commented on code in PR #1570:
URL: https://github.com/apache/phoenix/pull/1570#discussion_r1135066951
##########
pom.xml:
##########
@@ -98,7 +98,7 @@
<jackson-bom.version>2.12.6.20220326</jackson-bom.version>
<antlr.version>3.5.2</antlr.version>
<!-- Only used for tests with HBase 2.1-2.4 -->
- <reload4j.version>1.2.19</reload4j.version>
+ <reload4j.version>1.2.21</reload4j.version>
Review Comment:
I forgot that the issue was not with the compilation but the ITs. Without changing this version, I am getting the below exception:
```
java.lang.NoSuchFieldError: tlm
at org.apache.log4j.MDCFriend.fixForJava9(MDCFriend.java:11)
at org.slf4j.impl.Log4jMDCAdapter.<clinit>(Log4jMDCAdapter.java:38)
at org.slf4j.impl.StaticMDCBinder.getMDCA(StaticMDCBinder.java:59)
at org.slf4j.MDC.bwCompatibleGetMDCAdapterFromBinder(MDC.java:99)
at org.slf4j.MDC.<clinit>(MDC.java:108)
at org.apache.zookeeper.ClientCnxn$SendThread.startConnect(ClientCnxn.java:1080)
at org.apache.zookeeper.ClientCnxn$SendThread.run(ClientCnxn.java:1139)
```
This got fixed when I bumped up this version. Could you check if you are able to run this IT in IntelliJ with the old version?
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] tkhurana merged pull request #1570: PHOENIX-6821: Optimize batching in auto-commit mode
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana merged PR #1570:
URL: https://github.com/apache/phoenix/pull/1570
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] tkhurana commented on a diff in pull request #1570: PHOENIX-6821: Optimize batching in auto-commit mode
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #1570:
URL: https://github.com/apache/phoenix/pull/1570#discussion_r1131745100
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/UpsertValuesIT.java:
##########
@@ -480,10 +488,14 @@ public void testBatchedUpsert() throws Exception {
pstmt.addBatch();
pstmt.executeBatch();
conn.commit();
Review Comment:
```suggestion
if (!autocommit) {
conn.commit();
}
```
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] haridsv commented on a diff in pull request #1570: PHOENIX-6821: Optimize batching in auto-commit mode
Posted by "haridsv (via GitHub)" <gi...@apache.org>.
haridsv commented on code in PR #1570:
URL: https://github.com/apache/phoenix/pull/1570#discussion_r1135000724
##########
pom.xml:
##########
@@ -98,7 +98,7 @@
<jackson-bom.version>2.12.6.20220326</jackson-bom.version>
<antlr.version>3.5.2</antlr.version>
<!-- Only used for tests with HBase 2.1-2.4 -->
- <reload4j.version>1.2.19</reload4j.version>
+ <reload4j.version>1.2.21</reload4j.version>
Review Comment:
I was able to compile without this change, so reverted it.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] haridsv commented on a diff in pull request #1570: PHOENIX-6821: Optimize batching in auto-commit mode
Posted by "haridsv (via GitHub)" <gi...@apache.org>.
haridsv commented on code in PR #1570:
URL: https://github.com/apache/phoenix/pull/1570#discussion_r1138899059
##########
pom.xml:
##########
@@ -98,7 +98,7 @@
<jackson-bom.version>2.12.6.20220326</jackson-bom.version>
<antlr.version>3.5.2</antlr.version>
<!-- Only used for tests with HBase 2.1-2.4 -->
- <reload4j.version>1.2.19</reload4j.version>
+ <reload4j.version>1.2.21</reload4j.version>
Review Comment:
As it seems like a local env issue, I will keep this change out.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] haridsv commented on pull request #1570: PHOENIX-6821: Optimize batching in auto-commit mode
Posted by "haridsv (via GitHub)" <gi...@apache.org>.
haridsv commented on PR #1570:
URL: https://github.com/apache/phoenix/pull/1570#issuecomment-1495330597
@tkhurana They are all existing lines, not sure why they are getting flagged, but I just pushed a commit with fixes.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org