[jira] [Commented] (HDFS-14279) [SBN Read] Race condition in ObserverReadProxyProvider
[ https://issues.apache.org/jira/browse/HDFS-14279?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16775639#comment-16775639 ] Erik Krogen commented on HDFS-14279: Thanks for the review, [~shv]. I just committed this to trunk. > [SBN Read] Race condition in ObserverReadProxyProvider > -- > > Key: HDFS-14279 > URL: https://issues.apache.org/jira/browse/HDFS-14279 > Project: Hadoop HDFS > Issue Type: Bug > Components: hdfs, namenode >Reporter: Erik Krogen >Assignee: Erik Krogen >Priority: Major > Attachments: HDFS-14279.000.patch, HDFS-14279.001.patch > > > There is a race condition in {{ObserverReadProxyProvider#getCurrentProxy()}}: > {code} > private NNProxyInfo getCurrentProxy() { > if (currentProxy == null) { > changeProxy(null); > } > return currentProxy; > } > {code} > {{currentProxy}} is a {{volatile}}. Another {{changeProxy()}} could occur > after the {{changeProxy()}} and before the {{return}}, thus making the return > value incorrect. I have seen this result in an NPE. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-14279) [SBN Read] Race condition in ObserverReadProxyProvider
[ https://issues.apache.org/jira/browse/HDFS-14279?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16775644#comment-16775644 ] Hudson commented on HDFS-14279: --- SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #16033 (See [https://builds.apache.org/job/Hadoop-trunk-Commit/16033/]) HDFS-14279. [SBN read] Fix race condition in ObserverReadProxyProvider. (xkrogen: rev bad3ffd2907d75395907ff6b76c909ab50add4bc) * (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/server/namenode/ha/ObserverReadProxyProvider.java > [SBN Read] Race condition in ObserverReadProxyProvider > -- > > Key: HDFS-14279 > URL: https://issues.apache.org/jira/browse/HDFS-14279 > Project: Hadoop HDFS > Issue Type: Bug > Components: hdfs, namenode >Reporter: Erik Krogen >Assignee: Erik Krogen >Priority: Major > Fix For: 3.3.0 > > Attachments: HDFS-14279.000.patch, HDFS-14279.001.patch > > > There is a race condition in {{ObserverReadProxyProvider#getCurrentProxy()}}: > {code} > private NNProxyInfo getCurrentProxy() { > if (currentProxy == null) { > changeProxy(null); > } > return currentProxy; > } > {code} > {{currentProxy}} is a {{volatile}}. Another {{changeProxy()}} could occur > after the {{changeProxy()}} and before the {{return}}, thus making the return > value incorrect. I have seen this result in an NPE. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-14279) [SBN Read] Race condition in ObserverReadProxyProvider
[ https://issues.apache.org/jira/browse/HDFS-14279?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16775601#comment-16775601 ] Konstantin Shvachko commented on HDFS-14279: Looks great. +1 > [SBN Read] Race condition in ObserverReadProxyProvider > -- > > Key: HDFS-14279 > URL: https://issues.apache.org/jira/browse/HDFS-14279 > Project: Hadoop HDFS > Issue Type: Bug > Components: hdfs, namenode >Reporter: Erik Krogen >Assignee: Erik Krogen >Priority: Major > Attachments: HDFS-14279.000.patch, HDFS-14279.001.patch > > > There is a race condition in {{ObserverReadProxyProvider#getCurrentProxy()}}: > {code} > private NNProxyInfo getCurrentProxy() { > if (currentProxy == null) { > changeProxy(null); > } > return currentProxy; > } > {code} > {{currentProxy}} is a {{volatile}}. Another {{changeProxy()}} could occur > after the {{changeProxy()}} and before the {{return}}, thus making the return > value incorrect. I have seen this result in an NPE. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-14279) [SBN Read] Race condition in ObserverReadProxyProvider
[ https://issues.apache.org/jira/browse/HDFS-14279?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16774602#comment-16774602 ] Erik Krogen commented on HDFS-14279: [~shv], does the new patch look good to you? > [SBN Read] Race condition in ObserverReadProxyProvider > -- > > Key: HDFS-14279 > URL: https://issues.apache.org/jira/browse/HDFS-14279 > Project: Hadoop HDFS > Issue Type: Bug > Components: hdfs, namenode >Reporter: Erik Krogen >Assignee: Erik Krogen >Priority: Major > Attachments: HDFS-14279.000.patch, HDFS-14279.001.patch > > > There is a race condition in {{ObserverReadProxyProvider#getCurrentProxy()}}: > {code} > private NNProxyInfo getCurrentProxy() { > if (currentProxy == null) { > changeProxy(null); > } > return currentProxy; > } > {code} > {{currentProxy}} is a {{volatile}}. Another {{changeProxy()}} could occur > after the {{changeProxy()}} and before the {{return}}, thus making the return > value incorrect. I have seen this result in an NPE. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-14279) [SBN Read] Race condition in ObserverReadProxyProvider
[ https://issues.apache.org/jira/browse/HDFS-14279?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16773344#comment-16773344 ] Hadoop QA commented on HDFS-14279: -- | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 37s{color} | {color:blue} Docker mode activated. {color} | || || || || {color:brown} Prechecks {color} || | {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s{color} | {color:green} The patch does not contain any @author tags. {color} | | {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m 0s{color} | {color:red} The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. {color} | || || || || {color:brown} trunk Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 18m 4s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 47s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 23s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 45s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 12m 25s{color} | {color:green} branch has no errors when building and testing our client artifacts. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 40s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 29s{color} | {color:green} trunk passed {color} | || || || || {color:brown} Patch Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m 40s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 38s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 38s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 18s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 40s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m 0s{color} | {color:green} The patch has no whitespace issues. {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 12m 49s{color} | {color:green} patch has no errors when building and testing our client artifacts. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 46s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 26s{color} | {color:green} the patch passed {color} | || || || || {color:brown} Other Tests {color} || | {color:green}+1{color} | {color:green} unit {color} | {color:green} 1m 45s{color} | {color:green} hadoop-hdfs-client in the patch passed. {color} | | {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m 26s{color} | {color:green} The patch does not generate ASF License warnings. {color} | | {color:black}{color} | {color:black} {color} | {color:black} 55m 10s{color} | {color:black} {color} | \\ \\ || Subsystem || Report/Notes || | Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8f97d6f | | JIRA Issue | HDFS-14279 | | JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12959484/HDFS-14279.001.patch | | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle | | uname | Linux 491c7553a815 3.13.0-153-generic #203-Ubuntu SMP Thu Jun 14 08:52:28 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | /testptch/patchprocess/precommit/personality/provided.sh | | git revision | trunk / aa3ad36 | | maven | version: Apache Maven 3.3.9 | | Default Java | 1.8.0_191 | | findbugs | v3.1.0-RC1 | | Test Results | https://builds.apache.org/job/PreCommit-HDFS-Build/26272/testReport/ | | Max. process+thread count | 306 (vs. ulimit of 1) | | modules | C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client | | Console output | https://builds.apache.org/job/PreCommit-HDFS-Build/26272/console | | Powered by | Apache Yetus 0.8.0 http://yetus.apache.org | This message was automatically generated. > [SBN Read] Race condition in
[jira] [Commented] (HDFS-14279) [SBN Read] Race condition in ObserverReadProxyProvider
[ https://issues.apache.org/jira/browse/HDFS-14279?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16773276#comment-16773276 ] Erik Krogen commented on HDFS-14279: I ran some benchmarks offline and found that the use of {{volatile}} instead of {{synchronized}} wasn't helping performance at all. I ran with a client with 100/1000/5000 threads all doing {{getFileInfo()}} via the ObserverReadProxyProvider with my v000 (using volatile) and v001 (using synchronized always) patches and found no substantial speedup from v000. So, go with the simpler approach of always synchronizing. Attaching v001 patch accordingly. > [SBN Read] Race condition in ObserverReadProxyProvider > -- > > Key: HDFS-14279 > URL: https://issues.apache.org/jira/browse/HDFS-14279 > Project: Hadoop HDFS > Issue Type: Bug > Components: hdfs, namenode >Reporter: Erik Krogen >Assignee: Erik Krogen >Priority: Major > Attachments: HDFS-14279.000.patch, HDFS-14279.001.patch > > > There is a race condition in {{ObserverReadProxyProvider#getCurrentProxy()}}: > {code} > private NNProxyInfo getCurrentProxy() { > if (currentProxy == null) { > changeProxy(null); > } > return currentProxy; > } > {code} > {{currentProxy}} is a {{volatile}}. Another {{changeProxy()}} could occur > after the {{changeProxy()}} and before the {{return}}, thus making the return > value incorrect. I have seen this result in an NPE. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-14279) [SBN Read] Race condition in ObserverReadProxyProvider
[ https://issues.apache.org/jira/browse/HDFS-14279?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772079#comment-16772079 ] Erik Krogen commented on HDFS-14279: The intent is to avoid routing through a {{synchronized}} block unless it is necessary. In the common case, the field is currently non-null, and no synchronization is necessary. > [SBN Read] Race condition in ObserverReadProxyProvider > -- > > Key: HDFS-14279 > URL: https://issues.apache.org/jira/browse/HDFS-14279 > Project: Hadoop HDFS > Issue Type: Bug > Components: hdfs, namenode >Reporter: Erik Krogen >Assignee: Erik Krogen >Priority: Major > Attachments: HDFS-14279.000.patch > > > There is a race condition in {{ObserverReadProxyProvider#getCurrentProxy()}}: > {code} > private NNProxyInfo getCurrentProxy() { > if (currentProxy == null) { > changeProxy(null); > } > return currentProxy; > } > {code} > {{currentProxy}} is a {{volatile}}. Another {{changeProxy()}} could occur > after the {{changeProxy()}} and before the {{return}}, thus making the return > value incorrect. I have seen this result in an NPE. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-14279) [SBN Read] Race condition in ObserverReadProxyProvider
[ https://issues.apache.org/jira/browse/HDFS-14279?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16770635#comment-16770635 ] Konstantin Shvachko commented on HDFS-14279: I think we can simplify it mere: {code} private NNProxyInfo getCurrentProxy() { return changeProxy(null); } {code} > [SBN Read] Race condition in ObserverReadProxyProvider > -- > > Key: HDFS-14279 > URL: https://issues.apache.org/jira/browse/HDFS-14279 > Project: Hadoop HDFS > Issue Type: Bug > Components: hdfs, namenode >Reporter: Erik Krogen >Assignee: Erik Krogen >Priority: Major > Attachments: HDFS-14279.000.patch > > > There is a race condition in {{ObserverReadProxyProvider#getCurrentProxy()}}: > {code} > private NNProxyInfo getCurrentProxy() { > if (currentProxy == null) { > changeProxy(null); > } > return currentProxy; > } > {code} > {{currentProxy}} is a {{volatile}}. Another {{changeProxy()}} could occur > after the {{changeProxy()}} and before the {{return}}, thus making the return > value incorrect. I have seen this result in an NPE. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-14279) [SBN Read] Race condition in ObserverReadProxyProvider
[ https://issues.apache.org/jira/browse/HDFS-14279?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16769491#comment-16769491 ] Wei-Chiu Chuang commented on HDFS-14279: Looks good to me. I'd like to wait for Konstantin or Chao to review, since the code is delicate. > [SBN Read] Race condition in ObserverReadProxyProvider > -- > > Key: HDFS-14279 > URL: https://issues.apache.org/jira/browse/HDFS-14279 > Project: Hadoop HDFS > Issue Type: Bug > Components: hdfs, namenode >Reporter: Erik Krogen >Assignee: Erik Krogen >Priority: Major > Attachments: HDFS-14279.000.patch > > > There is a race condition in {{ObserverReadProxyProvider#getCurrentProxy()}}: > {code} > private NNProxyInfo getCurrentProxy() { > if (currentProxy == null) { > changeProxy(null); > } > return currentProxy; > } > {code} > {{currentProxy}} is a {{volatile}}. Another {{changeProxy()}} could occur > after the {{changeProxy()}} and before the {{return}}, thus making the return > value incorrect. I have seen this result in an NPE. -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-14279) [SBN Read] Race condition in ObserverReadProxyProvider
[ https://issues.apache.org/jira/browse/HDFS-14279?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16768588#comment-16768588 ] Hadoop QA commented on HDFS-14279: -- | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 17s{color} | {color:blue} Docker mode activated. {color} | || || || || {color:brown} Prechecks {color} || | {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s{color} | {color:green} The patch does not contain any @author tags. {color} | | {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m 0s{color} | {color:red} The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. {color} | || || || || {color:brown} trunk Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 16m 33s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 42s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 24s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 42s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 11m 55s{color} | {color:green} branch has no errors when building and testing our client artifacts. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 38s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 29s{color} | {color:green} trunk passed {color} | || || || || {color:brown} Patch Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m 40s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 35s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 35s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 18s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 39s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m 0s{color} | {color:green} The patch has no whitespace issues. {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 11m 52s{color} | {color:green} patch has no errors when building and testing our client artifacts. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 41s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 26s{color} | {color:green} the patch passed {color} | || || || || {color:brown} Other Tests {color} || | {color:green}+1{color} | {color:green} unit {color} | {color:green} 1m 47s{color} | {color:green} hadoop-hdfs-client in the patch passed. {color} | | {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m 26s{color} | {color:green} The patch does not generate ASF License warnings. {color} | | {color:black}{color} | {color:black} {color} | {color:black} 51m 36s{color} | {color:black} {color} | \\ \\ || Subsystem || Report/Notes || | Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8f97d6f | | JIRA Issue | HDFS-14279 | | JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12958746/HDFS-14279.000.patch | | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle | | uname | Linux b96a0c235be2 4.4.0-139-generic #165-Ubuntu SMP Wed Oct 24 10:58:50 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | /testptch/patchprocess/precommit/personality/provided.sh | | git revision | trunk / 0d7a5ac | | maven | version: Apache Maven 3.3.9 | | Default Java | 1.8.0_191 | | findbugs | v3.1.0-RC1 | | Test Results | https://builds.apache.org/job/PreCommit-HDFS-Build/26221/testReport/ | | Max. process+thread count | 441 (vs. ulimit of 1) | | modules | C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client | | Console output | https://builds.apache.org/job/PreCommit-HDFS-Build/26221/console | | Powered by | Apache Yetus 0.8.0 http://yetus.apache.org | This message was automatically generated. > [SBN Read] Race condition in