You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2022/06/15 03:43:26 UTC

[GitHub] [hadoop] ZanderXu opened a new pull request, #4441: HDFS-13522. IPC changes to support observer reads through routers.

ZanderXu opened a new pull request, #4441:
URL: https://github.com/apache/hadoop/pull/4441

   This PR is about RBF IPC changes in order to support Observer-Read.
   
   And this a draft PR, and relevant [PR](https://github.com/apache/hadoop/pull/4311) already exist.
   
   Different point:
   
   - Proxy always with AlignmentContext, like RouterGSIContext which contains a boolean flag, like enableObserverRead
   - Always update the lastSeenStateId from active
   - If enableObserverRead is false, updateRequestState will not set lastSeenStateId in RPCHeader
   - It's easily to dynamically reconfigure enableObserverRead.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] zhengchenyu commented on pull request #4441: HDFS-13522. IPC changes to support observer reads through routers.

Posted by GitBox <gi...@apache.org>.
zhengchenyu commented on PR #4441:
URL: https://github.com/apache/hadoop/pull/4441#issuecomment-1157672472

   > Thanks @zhengchenyu and @simbadzina .
   > 
   > > I think config in client side may be more flexible.
   > 
   > 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. But there may be some special Client that do not want to enable ObserverRead, so RBF should identify those requests and proxy them to the Active Namenode.
   > 
   > @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.
   > 
   > > In our draft design, after apply [HDFS-13522](https://issues.apache.org/jira/browse/HDFS-13522).002.patch, I wanna proxy client's state id.
   > 
   > Proxying client's state id to the NameNode by RBF will be very complicated.
   > 
   > * A DFSClient may read or write some paths of different NameServices, and the stateID of different NS may be different.
   > * The client does not know the Nameservice to which the reading or writing path belong, so it cannot pass the state id to RBF.
   
   Yes, you are right in some condition. If all client are common user, for hive and mr application, it is right. 
   We know observer can not guarantee strong consistency, maybe some use have high demand, they could wanna disable observe read, though few user have this demand.
   
   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.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] zhengchenyu commented on pull request #4441: HDFS-13522. IPC changes to support observer reads through routers.

Posted by GitBox <gi...@apache.org>.
zhengchenyu commented on PR #4441:
URL: https://github.com/apache/hadoop/pull/4441#issuecomment-1156087987

   @ZanderXu 
   Seems HDFS-13522.002.patch have include this function. The difference is that in HDFS-13522.002.patch, enable or disable observer read in client side. In your PR, enable or disable observer read in router side.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] ZanderXu commented on pull request #4441: HDFS-13522. IPC changes to support observer reads through routers.

Posted by GitBox <gi...@apache.org>.
ZanderXu commented on PR #4441:
URL: https://github.com/apache/hadoop/pull/4441#issuecomment-1157525096

   Thanks @zhengchenyu and @simbadzina .
   
   > I think config in client side may be more flexible.
   
   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. 
   But there may be some special Client that do not want to enable ObserverRead, so RBF should identify those requests and proxy them to the Active Namenode. 
   
   @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.
   
   > In our draft design, after apply [HDFS-13522](https://issues.apache.org/jira/browse/HDFS-13522).002.patch, I wanna proxy client's state id.
   
   Proxying client's state id to the NameNode by RBF will be very complicated. 
   - A DFSClient may read or write some paths of different NameServices, and the stateID of different NS may be different.
   - The client does not know the Nameservice to which the reading or writing path belong, so it cannot pass the state id to RBF.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] simbadzina commented on pull request #4441: HDFS-13522. IPC changes to support observer reads through routers.

Posted by GitBox <gi...@apache.org>.
simbadzina commented on PR #4441:
URL: https://github.com/apache/hadoop/pull/4441#issuecomment-1157784867

   > Thanks @zhengchenyu and @simbadzina .
   > 
   > > I think config in client side may be more flexible.
   > 
   > 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. But there may be some special Client that do not want to enable ObserverRead, so RBF should identify those requests and proxy them to the Active Namenode.
   > 
   > @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.
   > 
   > > In our draft design, after apply [HDFS-13522](https://issues.apache.org/jira/browse/HDFS-13522).002.patch, I wanna proxy client's state id.
   > 
   > Proxying client's state id to the NameNode by RBF will be very complicated.
   > 
   > * A DFSClient may read or write some paths of different NameServices, and the stateID of different NS may be different.
   > * The client does not know the Nameservice to which the reading or writing path belong, so it cannot pass the state id to RBF.
   
   @ZanderXu in my full PR, https://github.com/apache/hadoop/pull/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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] simbadzina commented on pull request #4441: HDFS-13522. IPC changes to support observer reads through routers.

Posted by GitBox <gi...@apache.org>.
simbadzina commented on PR #4441:
URL: https://github.com/apache/hadoop/pull/4441#issuecomment-1157792454

   @ZanderXu is https://github.com/apache/hadoop/pull/4127 I have configurations on both the router and client side. Consistency is also guaranteed because the router always does an msync.
   The reason for the client side configuration is for latency sensitive clients that just want one call between the router and the namenodes.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] ZanderXu commented on pull request #4441: HDFS-13522. IPC changes to support observer reads through routers.

Posted by GitBox <gi...@apache.org>.
ZanderXu commented on PR #4441:
URL: https://github.com/apache/hadoop/pull/4441#issuecomment-1157696846

   > We know observer can not guarantee strong consistency, maybe some use have high demand, they wanna disable observe read, though few user have this demand.
   
   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:
   1. Carry a special StateID to RBF, such as -100 (Client Process level)
   2. Carry a special filed attributes to RBF through CallerContext (single RPC level)
   3. etc..


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] ZanderXu commented on pull request #4441: HDFS-13522. IPC changes to support observer reads through routers.

Posted by GitBox <gi...@apache.org>.
ZanderXu commented on PR #4441:
URL: https://github.com/apache/hadoop/pull/4441#issuecomment-1157530100

   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] ZanderXu commented on pull request #4441: HDFS-13522. IPC changes to support observer reads through routers.

Posted by GitBox <gi...@apache.org>.
ZanderXu commented on PR #4441:
URL: https://github.com/apache/hadoop/pull/4441#issuecomment-1156018474

   Thanks @simbadzina @slfan1989 . Looking forward to your review results, thanks~


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] simbadzina commented on pull request #4441: HDFS-13522. IPC changes to support observer reads through routers.

Posted by GitBox <gi...@apache.org>.
simbadzina commented on PR #4441:
URL: https://github.com/apache/hadoop/pull/4441#issuecomment-1155957101

   > @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 is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] slfan1989 commented on pull request #4441: HDFS-13522. IPC changes to support observer reads through routers.

Posted by GitBox <gi...@apache.org>.
slfan1989 commented on PR #4441:
URL: https://github.com/apache/hadoop/pull/4441#issuecomment-1155969597

   This jira is very technically challenging, learn more from you!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] ZanderXu commented on pull request #4441: HDFS-13522. IPC changes to support observer reads through routers.

Posted by GitBox <gi...@apache.org>.
ZanderXu commented on PR #4441:
URL: https://github.com/apache/hadoop/pull/4441#issuecomment-1155952384

   @simbadzina @omalley @melissayou here, please help me review it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #4441: HDFS-13522. IPC changes to support observer reads through routers.

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4441:
URL: https://github.com/apache/hadoop/pull/4441#issuecomment-1156167007

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 56s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  1s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  1s |  |  detect-secrets was not available.  |
   | +0 :ok: |  xmllint  |   0m  1s |  |  xmllint was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  |  The patch appears to include 2 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |  15m 32s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  27m 49s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  25m  6s |  |  trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1  |
   | +1 :green_heart: |  compile  |  21m 29s |  |  trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  checkstyle  |   4m 34s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   3m 15s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   2m 42s |  |  trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1  |
   | +1 :green_heart: |  javadoc  |   2m 32s |  |  trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   4m 49s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  24m 53s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 27s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m 44s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  24m 10s |  |  the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1  |
   | +1 :green_heart: |  javac  |  24m 10s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  21m 42s |  |  the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  javac  |  21m 42s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | -0 :warning: |  checkstyle  |   4m 22s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4441/1/artifact/out/results-checkstyle-root.txt) |  root: The patch generated 2 new + 1 unchanged - 0 fixed = 3 total (was 1)  |
   | +1 :green_heart: |  mvnsite  |   3m  7s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   2m 37s |  |  the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1  |
   | +1 :green_heart: |  javadoc  |   2m 31s |  |  the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | -1 :x: |  spotbugs  |   2m  3s | [/new-spotbugs-hadoop-hdfs-project_hadoop-hdfs-rbf.html](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4441/1/artifact/out/new-spotbugs-hadoop-hdfs-project_hadoop-hdfs-rbf.html) |  hadoop-hdfs-project/hadoop-hdfs-rbf generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)  |
   | +1 :green_heart: |  shadedclient  |  25m  2s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  18m 44s |  |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  unit  |  41m 40s |  |  hadoop-hdfs-rbf in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   1m 18s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 289m 53s |  |  |
   
   
   | Reason | Tests |
   |-------:|:------|
   | SpotBugs | module:hadoop-hdfs-project/hadoop-hdfs-rbf |
   |  |  org.apache.hadoop.hdfs.server.federation.router.ConnectionManager.reconfEnableObserverRead(boolean) does not release lock on all exception paths  At ConnectionManager.java:on all exception paths  At ConnectionManager.java:[line 253] |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4441/1/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/4441 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets xmllint |
   | uname | Linux 99c2ac8f930f 4.15.0-166-generic #174-Ubuntu SMP Wed Dec 8 19:07:44 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 85596945ba77f639c3248f055cf7e8a0f7228ade |
   | Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4441/1/testReport/ |
   | Max. process+thread count | 2211 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-rbf U: . |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4441/1/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] ZanderXu commented on pull request #4441: HDFS-13522. IPC changes to support observer reads through routers.

Posted by GitBox <gi...@apache.org>.
ZanderXu commented on PR #4441:
URL: https://github.com/apache/hadoop/pull/4441#issuecomment-1156127801

   Thanks @zhengchenyu for your review and comment.  This a draft PR related to [PR4311](https://github.com/apache/hadoop/pull/4311).  I'm not following [HDFS-13522](https://issues.apache.org/jira/browse/HDFS-13522).002.patch, and I will read it carefully.
   
   Client -> RBF -> NameNode.
   Whether RBF proxies the read request to the Observer should have nothing to do with the Client.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] zhengchenyu commented on pull request #4441: HDFS-13522. IPC changes to support observer reads through routers.

Posted by GitBox <gi...@apache.org>.
zhengchenyu commented on PR #4441:
URL: https://github.com/apache/hadoop/pull/4441#issuecomment-1156196587

   > Thanks @zhengchenyu for your review and comment. This a draft PR related to [PR4311](https://github.com/apache/hadoop/pull/4311). I'm not following [HDFS-13522](https://issues.apache.org/jira/browse/HDFS-13522).002.patch, and I will read it carefully.
   > 
   > Client -> RBF -> NameNode. Whether RBF proxies the read request to the Observer should have nothing to do with the Client.
   
   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.
   I think config in client side may be more flexible.
   
   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.
   In our draft design, after apply HDFS-13522.002.patch, I wanna proxy client's state id. For busy work recently, I delay it. Maybe we can work and discuss it together.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] ZanderXu commented on pull request #4441: HDFS-13522. IPC changes to support observer reads through routers.

Posted by GitBox <gi...@apache.org>.
ZanderXu commented on PR #4441:
URL: https://github.com/apache/hadoop/pull/4441#issuecomment-1160217292

   Thanks @simbadzina @zhengchenyu for your review and warm discussion. I learned a lot from it. 
   I will close this draft pr and discuss this feature at [4311](https://github.com/apache/hadoop/pull/4311)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] ZanderXu closed pull request #4441: HDFS-13522. IPC changes to support observer reads through routers.

Posted by GitBox <gi...@apache.org>.
ZanderXu closed pull request #4441: HDFS-13522. IPC changes to support observer reads through routers.
URL: https://github.com/apache/hadoop/pull/4441


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] simbadzina commented on a diff in pull request #4441: HDFS-13522. IPC changes to support observer reads through routers.

Posted by GitBox <gi...@apache.org>.
simbadzina commented on code in PR #4441:
URL: https://github.com/apache/hadoop/pull/4441#discussion_r898588205


##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterGSIContext.java:
##########
@@ -0,0 +1,56 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdfs.server.federation.router;
+
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.hdfs.ClientGSIContext;
+import org.apache.hadoop.ipc.protobuf.RpcHeaderProtos.RpcRequestHeaderProto;
+
+/**
+ * Global State ID context for the router.
+ * <p>
+ * This is the router side implementation responsible for receiving
+ * state alignment info from server(s).
+ */
+@InterfaceAudience.Private
+@InterfaceStability.Evolving
+public class RouterGSIContext extends ClientGSIContext {

Review Comment:
   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?



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/ConnectionManager.java:
##########
@@ -231,6 +246,18 @@ public ConnectionContext getConnection(UserGroupInformation ugi,
     return conn;
   }
 
+  /**
+   * Dynamically reconfigure the enableObserverRead.
+   */
+  public void reconfEnableObserverRead(boolean enableObserverRead) {
+    readLock.lock();
+    this.enableObserverRead = enableObserverRead;
+    for (RouterGSIContext routerGSIContext : alignmentContexts.values()) {
+      routerGSIContext.setEnableObserverRead(enableObserverRead);
+    }
+    readLock.unlock();
+  }
+

Review Comment:
   Why do we need to reconfigure observer reads dynamically?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org