Skip to content

PHOENIX-6500 Allow 4.16 client to connect to 5.1 server - #1257

Merged
virajjasani merged 1 commit into
apache:masterfrom
virajjasani:PHOENIX-6500-master
Jun 30, 2021
Merged

PHOENIX-6500 Allow 4.16 client to connect to 5.1 server#1257
virajjasani merged 1 commit into
apache:masterfrom
virajjasani:PHOENIX-6500-master

Conversation

@virajjasani

Copy link
Copy Markdown
Contributor

No description provided.

@stoty

Copy link
Copy Markdown
Contributor

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec6m 27sDocker 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 💚mvninstall28m 43smaster passed
+0hbaserecompile37m 26sHBase recompiled.
+1 💚compile1m 17smaster passed
+1 💚checkstyle0m 50smaster passed
+1 💚javadoc1m 5smaster passed
+0 🆗spotbugs3m 39sphoenix-core in master has 965 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚mvninstall20m 18sthe patch passed
+0hbaserecompile32m 18sHBase recompiled.
+1 💚compile1m 15sthe patch passed
+1 💚javac1m 15sthe patch passed
-1 ❌checkstyle0m 49sphoenix-core: The patch generated 17 new + 655 unchanged - 1 fixed = 672 total (was 656)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc0m 59sthe patch passed
+1 💚spotbugs3m 58sthe patch passed
_ Other Tests _
-1 ❌unit123m 2sphoenix-core in the patch failed.
+1 💚asflicense0m 33sThe patch does not generate ASF License warnings.
213m 5s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1257/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#1257
Optional Testsdupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile
unameLinux 68620b6a1063 4.15.0-128-generic #131-Ubuntu SMP Wed Dec 9 06:57:35 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev/phoenix-personality.sh
git revisionmaster / fcdf5bc
Default JavaPrivate Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08
checkstylehttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1257/1/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
unithttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1257/1/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt
Test Resultshttps://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1257/1/testReport/
Max. process+thread count10514 (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-1257/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.


private static final Map<MajorMinorVersion, MajorMinorVersion> ALLOWED_SERVER_CLIENT_MAJOR_VERSION =
ImmutableMap.of(
new MajorMinorVersion(5, 1), new MajorMinorVersion(4, 16)

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.

Per https://phoenix.apache.org/upgrading.html

An older client (two minor versions back) will work with a newer server jar when the minor version is different, but not visa versa.
In other words, clients do not need to be upgraded in lock step with the server.

Does this mean you also need

new MajorMinorVersion(4, 15)
new MajorMinorVersion(4, 14)

? This 4.16 client might run against a 4.14 server before upgrade, or a 4.15 server before upgrade? If supporting clients two minor versions back.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Basically we have these validations:

  1. Client major and minor cannot be ahead of server
  2. Client major version must at least be up to server major version

What we are doing here is relaxing validation no 2. If we already know of client with old major version (4.x) being compatible to server with new major version (5.x), then we can relax major compatibility restriction (rule no 2) by adding such specific major/minor versions in this map.
In future, if we release 4.17 and we are aware that 4.17 client is backward compatible with 5.1 and 5.2 servers, we can add new entires in this map:

new MajorMinorVersion(5, 1), new MajorMinorVersion(4, 17) => 4.17 client can connect to 5.1 server
new MajorMinorVersion(5, 2), new MajorMinorVersion(4, 17) => 4.17 client can connect to 5.2 server
new MajorMinorVersion(5, 2), new MajorMinorVersion(4, 16) => 4.16 client can connect to 5.2 server

Without updating above entires, 4.17 client would not be able to connect to any 5.x server because rule no 2 (mentioned above) will fail this with:

java.sql.SQLException: ERROR 2006 (INT08): Incompatible jars detected between client and server. Major version of client is less than that of the server. Client version: 4.17.0; Server version: 5.1.0

This 4.16 client might run against a 4.14 server before upgrade, or a 4.15 server before upgrade?

This might never need to be supported unless I am missing something. So let's say we are running 4.14 client and server, now we upgrade server to 4.15 and client remains at 4.14, we are good. Instead of upgrading client, we again decide to upgrade server to 4.16, and we are still good because 4.14 client can connect to 4.16 server.
Before we upgrade server to 5.1, we will want to bring client to 4.16 so that 4.16 client can connect to 5.1 server.
However, in this entire case, we never need 4.16 client to run against 4.14/4.15 server. Correct?

@virajjasani
virajjasani merged commit e516b19 into apache:masterJun 30, 2021
@virajjasani
virajjasani deleted the PHOENIX-6500-master branch June 30, 2021 09:32
virajjasani added a commit that referenced this pull request Jun 30, 2021
Signed-off-by: Andrew Purtell <apurtell@apache.org>
virajjasani added a commit that referenced this pull request Jun 30, 2021
Signed-off-by: Andrew Purtell <apurtell@apache.org>
virajjasani added a commit that referenced this pull request Jun 30, 2021
Signed-off-by: Andrew Purtell <apurtell@apache.org>
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.

3 participants

@virajjasani@stoty@apurtell