-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
HDFS-13522. IPC changes to support observer reads through routers. #4441
Conversation
@simbadzina @omalley @melissayou here, please help me review it. |
Hi @ZanderXu. Sorry for the slow responses. Thanks for your review. I'll take a look at the draft PR and also your review comments tomorrow. |
This jira is very technically challenging, learn more from you! |
Thanks @simbadzina @slfan1989 . Looking forward to your review results, thanks~ |
@ZanderXu |
Thanks @zhengchenyu for your review and comment. This a draft PR related to PR4311. I'm not following HDFS-13522.002.patch, and I will read it carefully. Client -> RBF -> NameNode. |
💔 -1 overall
This message was automatically generated. |
In HDFS-13522.002.patch, isReadCall method, router will check "call.getClientStateId() == -1L". This is rpc call level. If observer read is disable in client side, call.getClientStateId() in router side will return -1, router will ignore observer namenode. By the way, I add some extra comment. In HDFS-13522.002.patch, router only check whether state id is -1. They don't pass client's state id. If dfs.federation.router.observer.auto-msync-period are not set to 0, but a large number, will be wrong. |
* Dynamically reconfigure the enableObserverRead. | ||
*/ | ||
public void reconfEnableObserverRead(boolean enableObserverRead) { | ||
readLock.lock(); | ||
this.enableObserverRead = enableObserverRead; | ||
for (RouterGSIContext routerGSIContext : alignmentContexts.values()) { | ||
routerGSIContext.setEnableObserverRead(enableObserverRead); | ||
} | ||
readLock.unlock(); | ||
} | ||
|
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.
Why do we need to reconfigure observer reads dynamically?
*/ | ||
@InterfaceAudience.Private | ||
@InterfaceStability.Evolving | ||
public class RouterGSIContext extends ClientGSIContext { |
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.
I feel the alignment between routers and namenode can be taken care of with just ClientGSIContext.
How does the router not updating the lastSeenStateID when communicating with the namenode help?
Thanks @zhengchenyu and @simbadzina .
This is a very meaningful topic. If only the client controls whether or not to enable ObserverRead will be more difficult for Admin to control, because it is very difficult to upgrade the HDFS client in full. In other words: If RBF controls whether the ObserverRead is enabled, the Admin will be very convenient to control the ObserverRead of the entire cluster, and even dynamically control whether the ObserverRead of a single NS or the entire cluster is enabled. @simbadzina This is why dynamic updates are required, so that when Admin finds that there are some abnormal Observer NameNodes, he/she can quickly disable the ObserverRead of one NS or even all NSs.
Proxying client's state id to the NameNode by RBF will be very complicated.
|
As in my draft PR above, RBF always updates lastSeenTxid from Active and saves. When an NS enable ObserverRead, RBF will set the stored lastSeenTxid of this NS to the RPC header and bring it to the Observer NameNode; if the NS disable ObserverRead, RBF will not set the stated id in RPC header, so even if the request is passed to the Observer, the Observer will also returns StandbyException. |
Yes, you are right in some condition. If all client are common user, for hive and mr application, it is right. Maybe we can reserve configuration both on router side and client side. Yes, Proxying client's state id is complicated. I don't know whether it is necessary or not. So just delay it. |
Only a very small number of users have high demand, and in most cases, the client enables ObserverRead default. In other words: In most cases, there is no need for client to pass the ObserverRead enable flag to RBF. So only a very small number of requests need to carry a specific flag bit to RBF, so that the RBF can force an msync to ensure the consistency before proxying the request. There are serval methods for the client side to carry the force consistency flag to RBF:
@zhengchenyu @simbadzina About my suggestion, I'm looking forward your feedback, thanks. In order to find a more perfect solution. |
@ZanderXu in my full PR, #4127, I do also allow routers to enable and disable observer reads. The difference being that it requires a router restart. Since routers are stateless this is a quick operation. At most one minute. |
Thanks @simbadzina @zhengchenyu for your review and warm discussion. I learned a lot from it. |
This PR is about RBF IPC changes in order to support Observer-Read.
And this a draft PR, and relevant PR already exist.
Different point: