Skip to content

PHOENIX-5224 Change 'Statement' to 'PreparedStatement' for better performance - #474

Open
bd2019us wants to merge 10 commits into
apache:masterfrom
bd2019us:PHOENIX-5224-PATCH
Open

PHOENIX-5224 Change 'Statement' to 'PreparedStatement' for better performance#474
bd2019us wants to merge 10 commits into
apache:masterfrom
bd2019us:PHOENIX-5224-PATCH

Conversation

@bd2019us

Copy link
Copy Markdown

Fix: #PHOENIX-5224. When the same SQL query is intensively used in a loop, it should be replaced with PreparedStatement, which is a big gain on performance.

stmt.executeUpdate(
"UPSERT INTO " + tableName + " VALUES('" + getRandomOrgId(maxOrgId) + "'," + i
+ "," + (i + 1) + "," + (i + 2) + ")");
stmt.setString(1, getRandomOrgId(maxOrgId);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing closing parenthesis.

conn.createStatement().execute("CREATE "+(localIndex? "LOCAL " : "")+"INDEX " + indexName + " ON " + fullTableName + "(val)");

conn.createStatement().execute("UPSERT INTO " + fullTableName + " VALUES (NEXT VALUE FOR " + seqName + ",1)");
PreparedStatement stmt = comm.prepareStatement("UPSERT INTO " + fullTableName + " SELECT NEXT VALUE FOR " + seqName + ", val FROM " + fullTableName);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

incorrect variable name.

@swaroopakswaroopak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your PR. This 'll be helpful once committed.
Although I see there are a few things missing that could have been caught by compiler/IDE. Does this PR compile properly?

@@ -104,9 +105,9 @@ public void testUpsertSelectDoesntSeeUpsertedData() throws Exception {
conn.createStatement().execute("CREATE "+(localIndex? "LOCAL " : "")+"INDEX " + indexName + " ON " + fullTableName + "(val)");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while you are at it, Could you please logically separate this line of code? Lots of things happening in one line.

Thanks.

@bd2019us

bd2019us commented Apr 7, 2019

Copy link
Copy Markdown
Author

@swaroopak Thanks for your reply. Sorry for my mistake and the compiling error is fixed, after which I run mvn compile and the output is shown below.

[INFO] Apache Phoenix ..................................... SUCCESS [ 0.564 s]
[INFO] Phoenix Core ....................................... SUCCESS [ 20.598 s]
[INFO] Phoenix - Pherf .................................... FAILURE [ 0.157 s]
[INFO] Phoenix Client ..................................... SKIPPED
[INFO] Phoenix Server ..................................... SKIPPED
[INFO] Phoenix Assembly ................................... SKIPPED
[INFO] Phoenix - Tracing Web Application .................. SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 21.960 s
[INFO] Finished at: 2019-04-07T11:17:44-05:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal on project phoenix-pherf: Could not resolve dependencies forproject org.apache.phoenix:phoenix-pherf:jar:5.1.0-HBase-2.0-SNAPSHOT: Failure to find org.apache.phoenix:phoenix-core:jar:tests:5.1.0-HBase-2.0-SNAPSHOTin https://repository.apache.org/content/repositories/releases/ was cached in the local repository, resolution will not be reattempted until the update interval of apache release has elapsed or updates are forced -> [Help 1]

Since I only changed the code in "Phoenix-core", I have no idea about the failure in "Phoenix - Pherf" , Could you kindly help. Thanks.

@swaroopak

Copy link
Copy Markdown
Contributor

make sure to have imported all the packages and run mvn clean install

@bd2019us

Copy link
Copy Markdown
Author

The compilation error is fixed. Could your please check?

[INFO] stopped o.e.j.s.h.ContextHandler{/webjars,file:/home/jiajun/empirical_pr/phoenix/}
[INFO] stopped o.e.j.s.h.ContextHandler{/classpath,file:/home/jiajun/empirical_pr/phoenix/}
[INFO] stopped o.e.j.s.h.ContextHandler{/,file:/home/jiajun/empirical_pr/phoenix/}
[INFO] stopped o.e.j.s.h.ContextHandler{/spec,file:/home/jiajun/empirical_pr/phoenix/}
[INFO] stopped o.e.j.s.h.ContextHandler{/src,file:/home/jiajun/empirical_pr/phoenix/}
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for Apache Phoenix 5.1.0-HBase-2.0-SNAPSHOT:
[INFO]
[INFO] Apache Phoenix ..................................... SUCCESS [ 2.114 s]
[INFO] Phoenix Core ....................................... SUCCESS [01:54 min]
[INFO] Phoenix - Pherf .................................... SUCCESS [ 13.054 s]
[INFO] Phoenix Client ..................................... SUCCESS [ 0.999 s]
[INFO] Phoenix Server ..................................... SUCCESS [ 2.929 s]
[INFO] Phoenix Assembly ................................... SUCCESS [ 7.306 s]
[INFO] Phoenix - Tracing Web Application .................. SUCCESS [ 11.253 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 02:35 min
[INFO] Finished at: 2019-04-11T15:32:37-05:00
[INFO] ------------------------------------------------------------------------

@swaroopak

Copy link
Copy Markdown
Contributor

@bd2019us could you as well submit the patch. So that nightly tests run on your patch?

@bd2019us

Copy link
Copy Markdown
Author

Sure @swaroopak , I updated the patch on JIRA

@gjacoby126

Copy link
Copy Markdown
Contributor

@bd2019us - changes look good - could you please rebase so that they can be committed?

@bd2019us

Copy link
Copy Markdown
Author

@gjacoby126 - thanks, we have updated the PR with rebase correspondingly, please check it

@stoty

Copy link
Copy Markdown
Contributor

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 1sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
+1 💚@author0m 0sThe patch does not contain any @author tags.
-1 ❌test4tests0m 0sThe patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ master Compile Tests _
+1 💚mvninstall23m 51smaster passed
+0hbaserecompile31m 1sHBase recompiled.
+1 💚compile1m 4smaster passed
+1 💚checkstyle1m 2smaster passed
+1 💚javadoc0m 52smaster passed
+0 🆗spotbugs3m 22sphoenix-core in master has 973 extant spotbugs warnings.
-0 ⚠️patch3m 30sUsed diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary.
_ Patch Compile Tests _
+1 💚mvninstall17m 2sthe patch passed
+0hbaserecompile29m 7sHBase recompiled.
+1 💚compile1m 5sthe patch passed
+1 💚javac1m 5sthe patch passed
+1 💚checkstyle1m 3sthe patch passed
-1 ❌whitespace0m 0sThe patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
+1 💚javadoc0m 51sthe patch passed
+1 💚spotbugs3m 34sthe patch passed
_ Other Tests _
+1 💚unit121m 13sphoenix-core in the patch passed.
+1 💚asflicense0m 15sThe patch does not generate ASF License warnings.
193m 22s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-474/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#474
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile
unameLinux 625cebae4600 4.15.0-147-generic #151-Ubuntu SMP Fri Jun 18 19:21:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev/phoenix-personality.sh
git revisionmaster / c4d297d
Default JavaPrivate Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08
whitespacehttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-474/1/artifact/yetus-general-check/output/whitespace-eol.txt
Test Resultshttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-474/1/testReport/
Max. process+thread count11400 (vs. ulimit of 30000)
modulesC: phoenix-core U: phoenix-core
Console outputhttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-474/1/console
versionsgit=2.7.4 maven=3.3.9 spotbugs=4.1.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@bd2019us@swaroopak@gjacoby126@stoty