HBASE-23304: RPCs needed for client meta information lookup #904
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
|
Next in the series of patches. This is a pretty simple one, just adds the needed RPCs and some sanity testing. @ndimiduk @apurtell @wchevreuil @saintstack @virajjasani FYI, you might be interested in reviewing this. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
ndimiduk
reviewed
Dec 10, 2019
There was a problem hiding this comment.
Just some minor comments on the code changes, so +1 from me as that goes.
However, I'd like to see a larger design discussion on this API. It's going to be in the critical path of data, so it's worth taking a critical look at how this API will be used and if each RPC is necessary. Do you have this written up in a design doc someplace? The API definitions and their semantics fall under the category of "Client-Server Wire Compatibility" as described in our book, http://hbase.apache.org/book.html#hbase.versioning.post10
| /** | ||
| * Implements all the RPCs needed by clients to look up cluster meta information needed for connection establishment. | ||
| */ | ||
| service ClientMetaService { |
There was a problem hiding this comment.
Not saying it's right or wrong (yet), but why define a new service rather than add these methods to the existing "MasterService" ?
There was a problem hiding this comment.
It just seemed logical to me to group these (and possibly related RPCs that we add in the future) into a separate service that master is just implementing today. Theoretically they are not tied to masters in any way. Any other process can implement it too (like you already mentioned in the jira comments of HBASE-18095). What do you think?
There was a problem hiding this comment.
I like the idea of a separate service. If someone wants to move this later.
| import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetMetaRegionLocationsRequest; | ||
| import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetMetaRegionLocationsResponse; | ||
|
|
||
| @Category({SmallTests.class, MasterTests.class}) |
There was a problem hiding this comment.
This needs to be at least a MediumTests because it spins up a mini-cluster at all. Have a look at the java docs on those annotations for how they categorize things.
|
💔 -1 overall
This message was automatically generated. |
|
@ndimiduk I actually missed your comment on the compatibility. I documented them in the jira for future, but I'm pasting them here for convenience. Following is the information needed by clients during connection init and the corresponding RPC endpoints added by the patch.
Given these are new service endpoints, we need to consider some upgrade scenarios.
Given this compatibility matrix,
|
|
@ndimiduk There has been some discussion over the design doc [1] but I don't see any major blocking comments from anyone. If that seems ok, can you please merge this patch? (I already addressed the test annotation issue) [1] https://docs.google.com/document/d/1JAJdM7eUxg5b417f0xWS4NztKCx1f2b6wZrudPtiXF4 |
asfgit
force-pushed
the
HBASE-18095/client-locate-meta-no-zookeeper
branch
2 times, most recently
from
e64830d to
361a3c1
Compare
This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches.
bharathv
force-pushed
the
HBASE-23304
branch
from
0e7c9f5 to
6fd9ac8
Compare
|
@apurtell This is the change I was talking about in the offline chat. This is currently a blocker for HBASE-23305. Just putting this in your radar. To give you some context, @ndimiduk and I were chatting about this offline and he suggested that I get your feedback before merging. Appreciate any comments on the overall design [1]. Thanks. [1] https://docs.google.com/document/d/1JAJdM7eUxg5b417f0xWS4NztKCx1f2b6wZrudPtiXF4/edit |
|
💔 -1 overall
This message was automatically generated. |
apurtell
approved these changes
Dec 19, 2019
| /** | ||
| * Implements all the RPCs needed by clients to look up cluster meta information needed for connection establishment. | ||
| */ | ||
| service ClientMetaService { |
There was a problem hiding this comment.
I like the idea of a separate service. If someone wants to move this later.
| GetActiveMasterRequest request) throws ServiceException { | ||
| GetActiveMasterResponse.Builder resp = GetActiveMasterResponse.newBuilder(); | ||
| Optional<ServerName> serverName = master.getActiveMaster(); | ||
| serverName.ifPresent(name -> resp.setServerName(ProtobufUtil.toServerName(name))); |
There was a problem hiding this comment.
These Java 8 constructs don't add much and make backports harder. No issues here, just a thought.
There was a problem hiding this comment.
Ya, will clean this up in the back ports separately.
| GetMetaRegionLocationsResponse.Builder response = GetMetaRegionLocationsResponse.newBuilder(); | ||
| Optional<List<HRegionLocation>> metaLocations = | ||
| master.getMetaRegionLocationCache().getMetaRegionLocations(); | ||
| metaLocations.ifPresent(hRegionLocations -> hRegionLocations.forEach( |
| import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetMetaRegionLocationsRequest; | ||
| import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetMetaRegionLocationsResponse; | ||
|
|
||
| @Category({MediumTests.class, MasterTests.class}) |
There was a problem hiding this comment.
Add a unit that checks that all running masters successfully answer? Will catch the impact if someone changes something in the future that prevents standby masters from answering.
There was a problem hiding this comment.
Let me clarify. You loop over the masters but don't assert that all masters answer exactly
|
So for now you would only want this merged into apache:HBASE-18095/client-locate-meta-no-zookeeper ? @bharathv |
Adds an assert on the total RPC count for each test.
Yes, please. Will do the backports together once all the feature patches land in master. |
|
💔 -1 overall
This message was automatically generated. |
bharathv
added a commit
to bharathv/hbase
that referenced
this pull request
Dec 20, 2019
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]>
asfgit
pushed a commit
that referenced
this pull request
Dec 20, 2019
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]>
asfgit
pushed a commit
that referenced
this pull request
Dec 28, 2019
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]>
I don't see this covered in the design doc at all. I was hoping to see some kind of state diagram that show the happy-path and error-case RPC calls made during client initialization, making use of these 3 new endpoints. Are there places where we can reduce the necessary RPCs? For example, how about a removing "get active master" and just have the client call "get meta locations". If the client happens to be communicating with the active master, it's saved an RPC. If it's not the active master, the failure response can contain the location of the active master. For reference, consider an HTTP 301 response. |
I'm a little confused. With patches like #812 #830 etc all the running masters (active/stand-by) can serve the information needed by a connection registry. For example: stand-by masters keep track of active masters so that they can serve the "get active master" request. Client does not need to keep retrying until it finds an active master. On top of that we now also have the ability to hedge requests across masters (#954). Did I miss something? |
asfgit
pushed a commit
that referenced
this pull request
Jan 3, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]>
Likely no, it's me who's missing something, and probably other more "casual" reviewers as well; thus my request for some description of the protocol for invoking these new RPC endpoints. I have the apparently outdated understanding that the location of the meta region(s) is only served by the active master. Thanks for your patience. |
|
Cool, thanks for taking a look, appreciate the reviews! |
asfgit
pushed a commit
that referenced
this pull request
Jan 29, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]>
asfgit
pushed a commit
that referenced
this pull request
Jan 30, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]>
asfgit
pushed a commit
that referenced
this pull request
Feb 3, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]>
ndimiduk
added a commit
to ndimiduk/hbase
that referenced
this pull request
Feb 3, 2020
…pache#904) (apache#1098)" This reverts commit 71f0354.
ndimiduk
added a commit
to ndimiduk/hbase
that referenced
this pull request
Feb 3, 2020
…pache#904) (apache#1098)" This reverts commit 71f0354.
asfgit
pushed a commit
that referenced
this pull request
Feb 4, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]>
asfgit
pushed a commit
that referenced
this pull request
Feb 5, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]>
asfgit
pushed a commit
that referenced
this pull request
Feb 5, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]>
bharathv
added a commit
that referenced
this pull request
Feb 9, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]>
bharathv
added a commit
that referenced
this pull request
Feb 11, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]>
bharathv
added a commit
that referenced
this pull request
Feb 13, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]>
bharathv
added a commit
to bharathv/hbase
that referenced
this pull request
Feb 14, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]>
bharathv
added a commit
to bharathv/hbase
that referenced
this pull request
Feb 17, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]>
bharathv
added a commit
that referenced
this pull request
Feb 18, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]>
bharathv
added a commit
to bharathv/hbase
that referenced
this pull request
Feb 20, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]>
bharathv
added a commit
that referenced
this pull request
Feb 20, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]>
bharathv
added a commit
to bharathv/hbase
that referenced
this pull request
Feb 23, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]> (cherry picked from commit 4f8fbba)
bharathv
added a commit
to bharathv/hbase
that referenced
this pull request
Feb 25, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]> (cherry picked from commit 4f8fbba)
bharathv
added a commit
to bharathv/hbase
that referenced
this pull request
Feb 26, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]> (cherry picked from commit 4f8fbba)
bharathv
added a commit
that referenced
this pull request
Feb 27, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]> (cherry picked from commit 4f8fbba)
thangTang
pushed a commit
to thangTang/hbase
that referenced
this pull request
Apr 16, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]>
thangTang
pushed a commit
to thangTang/hbase
that referenced
this pull request
Apr 16, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]>
bharathv
added a commit
to bharathv/hbase
that referenced
this pull request
Aug 20, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]> (cherry picked from commit 4f8fbba) (cherry picked from commit 488460e)
bharathv
added a commit
to bharathv/hbase
that referenced
this pull request
Aug 21, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]> (cherry picked from commit 4f8fbba) (cherry picked from commit 488460e)
bharathv
added a commit
to bharathv/hbase
that referenced
this pull request
Aug 21, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]> (cherry picked from commit 4f8fbba) (cherry picked from commit 488460e)
bharathv
added a commit
to bharathv/hbase
that referenced
this pull request
Sep 1, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]> (cherry picked from commit 4f8fbba) (cherry picked from commit 488460e)
bharathv
added a commit
to bharathv/hbase
that referenced
this pull request
Sep 18, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]> (cherry picked from commit 4f8fbba) (cherry picked from commit 488460e)
bharathv
added a commit
that referenced
this pull request
Sep 19, 2020
* HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <[email protected]> (cherry picked from commit 4f8fbba) (cherry picked from commit 488460e)
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 patch implements the RPCs needed for the meta information
lookup during connection init. New tests added to cover the RPC
code paths. HBASE-23305 builds on this to implement the client
side logic.
Fixed a bunch of checkstyle nits around the places the patch
touches.