PHOENIX-6821: Optimize batching in auto-commit mode #1570
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tkhurana
requested changes
Mar 9, 2023
pom.xml
Outdated
| @@ -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> | |||
There was a problem hiding this comment.
This doesn't seem related to this PR
There was a problem hiding this 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.
There was a problem hiding this comment.
I was able to compile without this change, so reverted it.
There was a problem hiding this 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?
There was a problem hiding this comment.
As it seems like a local env issue, I will keep this change out.
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java
Show resolved
Hide resolved
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java
Outdated
Show resolved
Hide resolved
tkhurana
requested changes
Mar 9, 2023
phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionCode.java
Outdated
Show resolved
Hide resolved
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java
Outdated
Show resolved
Hide resolved
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java
Show resolved
Hide resolved
tkhurana
requested changes
Mar 9, 2023
tkhurana
reviewed
Mar 9, 2023
tkhurana
reviewed
Mar 9, 2023
phoenix-core/src/it/java/org/apache/phoenix/end2end/UpsertValuesIT.java
Outdated
Show resolved
Hide resolved
tkhurana
approved these changes
Mar 20, 2023
haridsv
added a commit
to haridsv/phoenix
that referenced
this pull request
Mar 30, 2023
Squash of the change from PR apache#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. - Replace the use of custom BatchUpdateExecution exception with the java.sql.BatchUpdateException and propagate update counts through it. - Add unit test coverage for executeBatch() - Add tests for delete and further enhance existing tests - Review feedback changes Squashes the following commits to avoid patch errors during PR validation builds: * b9eb9bf Addressed additional review feedback * 54973f7 Add tests for delete and further enhance existing tests * 7548551 Restoring prior version for reload4j.version * b29d8d2 Add unit test coverage for executeBatch() * 6e0c825 Review feedback changes * f129a15 Reverted the code that prevents mixing of non-batch commit after a batch commit as it is broken * af39750 Fix checkstyle errors in the lines touched in this PR. * 7236deb Merge branch 'master' into PHOENIX-6821 * 5c6ac93 PHOENIX-6821: Optimize batching in auto-commit mode
tkhurana
pushed a commit
that referenced
this pull request
Apr 3, 2023
* PHOENIX-6821: Optimize batching in auto-commit mode Squash of the change from PR #1570 * Fix a merge issue with cherry-pick * Additional changes needed to fix compilation errors for 5.1 branch * Fix checkstyle errors * Fix checkstyle errors * Remove extraneous whitespace
|
@haridsv There are still some checkstyle warnings left. |
|
@tkhurana They are all existing lines, not sure why they are getting flagged, but I just pushed a commit with fixes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change also includes the following to make the batch functionality more consistent and confirmant to the JDBC spec:
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.