Skip to content

Conversation

@PDavid
Copy link
Contributor

@PDavid PDavid commented Sep 18, 2024

https://issues.apache.org/jira/browse/HBASE-28645

Add version and revision fields to the /version/rest endpoint, which reports the build version and Git revision of the REST server component.

Version determination

The REST server component determines the version and revision locally (i.e. not from the HBase cluster).

The REST server component uses the org.apache.hadoop.hbase.util.VersionInfo class (from the hbase-commons project).
So when the REST server component (hbase-rest-*.jar) is running it gets the version and revision from hbase-commons-*.jar on the classpath.

Startup logging

Note: The version and revision is logged already when the REST server component is started and it already uses the same org.apache.hadoop.hbase.util.VersionInfo class, so I did not changed this.

$ bin/hbase rest start -p 8080
2024-09-18T16:28:22,357 INFO  [main {}] RESTServer: ***** STARTING service 'RESTServer' *****
2024-09-18T16:28:22,361 INFO  [main {}] util.VersionInfo: HBase 4.0.0-alpha-1-SNAPSHOT
2024-09-18T16:28:22,361 INFO  [main {}] util.VersionInfo: Source code repository git://<redacted> revision=8f9efaf4977c99cb65aaf54fe1ec2a250e9c16fe
2024-09-18T16:28:22,361 INFO  [main {}] util.VersionInfo: Compiled by david on Wed 18 Sep 16:20:58 CEST 2024
2024-09-18T16:28:22,361 INFO  [main {}] util.VersionInfo: From source with checksum 9e59963a389d1db71a0bdab4fee15bb02c0e86bb39b8ffb3e784ca730ca7e6f33ba97da86acb2d62d2d1fe1795c223b96c8816efaa2dc6a88cd6deb3d1c98e4c

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

optional string osVersion = 3;
optional string serverVersion = 4;
optional string jerseyVersion = 5;
optional string version = 6;
Copy link
Contributor

Choose a reason for hiding this comment

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

Are older clients going to be able to handle this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Many thanks, this is a very good question. I'll investigate this a bit to make sure.

Copy link
Contributor Author

@PDavid PDavid Sep 20, 2024

Choose a reason for hiding this comment

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

The short answer is: yes.

The long answer:

Extending a Protocol Buffer

Sooner or later after you release the code that uses your protocol buffer, you will undoubtedly want to “improve” the protocol buffer’s definition. If you want your new buffers to be backwards-compatible, and your old buffers to be forward-compatible – and you almost certainly do want this – then there are some rules you need to follow. In the new version of the protocol buffer:

  • you must not change the tag numbers of any existing fields.
  • you must not add or delete any required fields.
  • you may delete optional or repeated fields.
  • you may add new optional or repeated fields but you must use fresh tag numbers (that is, tag numbers that -
    were never used in this protocol buffer, not even by deleted fields).
    (There are some exceptions to these rules, but they are rarely used.)

If you follow these rules, old code will happily read new messages and simply ignore any new fields. To the old code, optional fields that were deleted will simply have their default value, and deleted repeated fields will be empty. New code will also transparently read old messages. However, keep in mind that new optional fields will not be present in old messages, so you will need to either check explicitly whether they’re set with has_, or provide a reasonable default value in your .proto file with [default = value] after the tag number. If the default value is not specified for an optional element, a type-specific default value is used instead: for strings, the default value is the empty string.
...

Source: https://protobuf.dev/getting-started/javatutorial/#extending-a-protobuf

Besides I tested it like this:

  • Took (copied) the Base64 encoded serialized protobuf message which also contains the newly added version and revision fields:
AS_PB = "CgUwLjAuMRInU3VuIE1pY3Jvc3lzdGVtcyBJbmMuIDEuNi4wXzEzLTExLjMtYjAyGi1MaW51eCAyLjYuMTg"
      + "tMTI4LjEuNi5lbDUuY2VudG9zLnBsdXN4ZW4gYW1kNjQiBjYuMS4xNCoIMS4xLjAtZWEyFjQuMC4wLWFscGhhLT"
      + "EtU05BUFNIT1Q6KDUwODVkMjdhYjE3ZDg1NzExOGE5NmFlM2YzN2MwMGI2MGM5MjU0NzE=";
  • Switched to the master branch (which does not yet have these version and revision fields yet).
  • In TestVersionModel I changed the serialized protobuf string to the new one which has the new fields
  • Run a Maven clean install to make sure the project is clean
  • Executed TestVersionModel unit test which was green (was able to read the protobuf message).

(I also tried out this in a small new Java project.)

--> older clients should read new message and newer clients should read old messages without a problem.

@PDavid PDavid marked this pull request as ready for review September 19, 2024 08:00
@PDavid
Copy link
Contributor Author

PDavid commented Sep 20, 2024

In the meantime I found that the Reference Guide on the https://hbase.apache.org/ website also lists the protobuf and XML schemas which needs to be updated:

Will do that quickly.

@stoty
Copy link
Contributor

stoty commented Sep 20, 2024

Thank you.
However, I believe that you need to make the new fields optional, otherwise the new client won't be able to read the old server's response.
Please test that case as well.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@PDavid
Copy link
Contributor Author

PDavid commented Sep 20, 2024

Thank you. However, I believe that you need to make the new fields optional, otherwise the new client won't be able to read the old server's response. Please test that case as well.

Thanks, good point. 👍 I tested multiple cases now, including the one you mentioned and every case works and there are no errors.

Created a small and dirty Java application which calls the rest version REST API endpoint of the HBase REST server component, then parses the response to the VersionModel HBase REST model class. The application requests the following representations:

  • protobuf
  • XML
  • JSON

Java code: https://github.com/PDavid/hbase-rest-client/blob/master/src/main/java/hu/paksyd/hbase/restclient/Main.java

Test results

(New version means this branch.)

New REST server version - old client (2.5.9)

Protobuf:
GET /version/rest
200 OK

VersionModel: rest 0.0.3 [JVM: Eclipse Adoptium 17.0.12-17.0.12+7] [OS: Linux 6.8.0-45-generic amd64] [Server: jetty/9.4.55.v20240627] [Jersey: ]

XML:
GET /version/rest
200 OK

VersionModel: rest 0.0.3 [JVM: Eclipse Adoptium 17.0.12-17.0.12+7] [OS: Linux 6.8.0-45-generic amd64] [Server: jetty/9.4.55.v20240627] [Jersey: ]

JSON:
GET /version/rest
200 OK

VersionModel: rest 0.0.3 [JVM: Eclipse Adoptium 17.0.12-17.0.12+7] [OS: Linux 6.8.0-45-generic amd64] [Server: jetty/9.4.55.v20240627] [Jersey: ]

New REST server version - new client

Protobuf:
GET /version/rest
200 OK

VersionModel: rest 0.0.3 [JVM: Eclipse Adoptium 17.0.12-17.0.12+7] [OS: Linux 6.8.0-45-generic amd64] [Server: jetty/9.4.55.v20240627] [Jersey: ] [Version: 4.0.0-alpha-2-SNAPSHOT] [Revision: de635766aea8113241a3f04f373d0dea36d848f7]

XML:
GET /version/rest
200 OK

VersionModel: rest 0.0.3 [JVM: Eclipse Adoptium 17.0.12-17.0.12+7] [OS: Linux 6.8.0-45-generic amd64] [Server: jetty/9.4.55.v20240627] [Jersey: ] [Version: 4.0.0-alpha-2-SNAPSHOT] [Revision: de635766aea8113241a3f04f373d0dea36d848f7]

JSON:
GET /version/rest
200 OK

VersionModel: rest 0.0.3 [JVM: Eclipse Adoptium 17.0.12-17.0.12+7] [OS: Linux 6.8.0-45-generic amd64] [Server: jetty/9.4.55.v20240627] [Jersey: ] [Version: 4.0.0-alpha-2-SNAPSHOT] [Revision: de635766aea8113241a3f04f373d0dea36d848f7]

Old REST server version (2.5.9) - new client

Protobuf:
GET /version/rest
200 OK

VersionModel: rest 0.0.3 [JVM: Ubuntu 11.0.24-11.0.24+8-post-Ubuntu-1ubuntu324.04.1] [OS: Linux 6.8.0-45-generic amd64] [Server: jetty/9.4.52.v20230823] [Jersey: ] [Version: null] [Revision: null]

XML:
GET /version/rest
200 OK

VersionModel: rest 0.0.3 [JVM: Ubuntu 11.0.24-11.0.24+8-post-Ubuntu-1ubuntu324.04.1] [OS: Linux 6.8.0-45-generic amd64] [Server: jetty/9.4.52.v20230823] [Jersey: ] [Version: null] [Revision: null]

JSON:
GET /version/rest
200 OK

VersionModel: rest 0.0.3 [JVM: Ubuntu 11.0.24-11.0.24+8-post-Ubuntu-1ubuntu324.04.1] [OS: Linux 6.8.0-45-generic amd64] [Server: jetty/9.4.52.v20230823] [Jersey: ] [Version: null] [Revision: null]

@PDavid PDavid requested a review from stoty September 20, 2024 15:07
@PDavid
Copy link
Contributor Author

PDavid commented Sep 20, 2024

I also noticed that that this GET /version/rest API endpoint is not documented in the Reference Guide on the website.

Should I maybe document it there?

@stoty
Copy link
Contributor

stoty commented Sep 20, 2024

That would be nice

@NihalJain
Copy link
Contributor

I also noticed that that this GET /version/rest API endpoint is not documented in the Reference Guide on the website.

Should I maybe document it there?

Hi @PDavid You may want to have a look at https://issues.apache.org/jira/browse/HBASE-20714. Feel free to assign this to youself along with the version endpoint documentation task.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 50s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 buf 0m 0s buf was not available.
+0 🆗 buf 0m 0s buf was not available.
+0 🆗 codespell 0m 0s codespell was not available.
+0 🆗 detsecrets 0m 0s detect-secrets was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
+1 💚 hbaseanti 0m 0s Patch does not have any anti-patterns.
_ master Compile Tests _
+0 🆗 mvndep 0m 12s Maven dependency ordering for branch
+1 💚 mvninstall 4m 8s master passed
+1 💚 compile 13m 21s master passed
+1 💚 checkstyle 1m 43s master passed
+1 💚 spotbugs 16m 2s master passed
+0 🆗 refguide 3m 39s branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect.
+1 💚 spotless 1m 9s branch has no errors when running spotless:check.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 12s Maven dependency ordering for patch
+1 💚 mvninstall 3m 49s the patch passed
+1 💚 compile 12m 52s the patch passed
+1 💚 cc 12m 52s the patch passed
+1 💚 javac 12m 52s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 checkstyle 1m 58s the patch passed
+1 💚 spotbugs 18m 24s the patch passed
+0 🆗 refguide 4m 3s patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect.
+1 💚 hadoopcheck 12m 19s Patch does not cause any errors with Hadoop 3.3.6 3.4.0.
+1 💚 hbaseprotoc 3m 29s the patch passed
+1 💚 spotless 0m 52s patch has no errors when running spotless:check.
_ Other Tests _
+1 💚 asflicense 0m 29s The patch does not generate ASF License warnings.
107m 22s
Subsystem Report/Notes
Docker ClientAPI=1.47 ServerAPI=1.47 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6262/4/artifact/yetus-general-check/output/Dockerfile
GITHUB PR #6262
Optional Tests dupname asflicense cc buflint bufcompat codespell detsecrets hbaseprotoc spotless javac spotbugs checkstyle compile hadoopcheck hbaseanti refguide
uname Linux 054f8e4a419b 5.4.0-195-generic #215-Ubuntu SMP Fri Aug 2 18:28:05 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / 4c09f3b
Default Java Eclipse Adoptium-17.0.11+9
refguide https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-6262/4/yetus-general-check/output/branch-site/book.html
refguide https://nightlies.apache.org/hbase/HBase-PreCommit-GitHub-PR/PR-6262/4/yetus-general-check/output/patch-site/book.html
Max. process+thread count 190 (vs. ulimit of 30000)
modules C: hbase-protocol-shaded hbase-rest . U: .
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6262/4/console
versions git=2.34.1 maven=3.9.8 spotbugs=4.7.3
Powered by Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 50s Docker mode activated.
-0 ⚠️ yetus 0m 3s Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+0 🆗 mvndep 0m 28s Maven dependency ordering for branch
+1 💚 mvninstall 5m 2s master passed
+1 💚 compile 3m 31s master passed
+1 💚 javadoc 3m 59s master passed
+1 💚 shadedjars 8m 17s branch has no errors when building our shaded downstream artifacts.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 13s Maven dependency ordering for patch
+1 💚 mvninstall 4m 27s the patch passed
+1 💚 compile 3m 22s the patch passed
+1 💚 javac 3m 22s the patch passed
+1 💚 javadoc 3m 38s the patch passed
+1 💚 shadedjars 7m 29s patch has no errors when building our shaded downstream artifacts.
_ Other Tests _
+1 💚 unit 304m 56s root in the patch passed.
353m 37s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6262/4/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR #6262
Optional Tests unit javac javadoc compile shadedjars
uname Linux c84aa5c2f73f 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / 4c09f3b
Default Java Eclipse Adoptium-17.0.11+9
Test Results https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6262/4/testReport/
Max. process+thread count 8768 (vs. ulimit of 30000)
modules C: hbase-protocol-shaded hbase-rest . U: .
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6262/4/console
versions git=2.34.1 maven=3.9.8
Powered by Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

Copy link
Contributor

@stoty stoty left a comment

Choose a reason for hiding this comment

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

+1 LGTM

Thanks for thoroughly testing my concerns.

@stoty
Copy link
Contributor

stoty commented Sep 26, 2024

Please create a separate PR for branch-2, so that the CI checks can run on that one, too.

@PDavid
Copy link
Contributor Author

PDavid commented Sep 26, 2024

Many thanks for both for your feedback. 👍

Please create a separate PR for branch-2, so that the CI checks can run on that one, too.

I created the backport PR here:
#6309

@stoty stoty merged commit 64a49a2 into apache:master Sep 27, 2024
stoty pushed a commit that referenced this pull request Sep 27, 2024
…#6262)

Signed-off-by: Nihal Jain <[email protected]>
Signed-off-by: Istvan Toth <[email protected]>
(cherry picked from commit 64a49a2)
@PDavid PDavid deleted the HBASE-28645 branch September 27, 2024 13:28
Sign up for free to 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