-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HBASE-25070 : With new generic API getLogEntries, cleaning up unused RPC APIs #2426
Conversation
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Remove public API? When this API was added? |
Not exactly public API, it's our internal RPC API. New RPC API was added as part of HBASE-24528 and is applicable to 2.4+ only. So this change is also planned for master and branch-2 only. |
@infraio this sounds good to you? |
Got it. The client api was implemented by new rpc. These not used anymore. But one problem is: the old client cannot access new server? |
Old client for this use-case would be 2.3.x releases and yes they would not be able to access new RPC. So the question is this might be an issue for any deprecated RPC API right? Our Do we follow any guidelines for Rpc implementations w.r.t client compatibility? e.g not remove deprecated RPCs from |
I think old client capable of using old RPC should be a valid case of minor releases, let me just target for master branch for this change. Thought @infraio ? |
bq. Do we follow any guidelines for Rpc implementations w.r.t client compatibility? e.g not remove deprecated RPCs from Master.proto and admin.proto in minor releases? This is descripted in http://hbase.apache.org/book.html#hbase.versioning.post10 . This is about "Client-Server wire protocol compatibility". And it can be breaked between major version. "Only land to master branch" is ok. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 only for master branch.
Thanks @infraio . Wondering if we should change |
No description provided.