Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-2092

Create a light inner conf class in DFSClient

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.23.0
    • Fix Version/s: 0.23.0
    • Component/s: hdfs-client
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      At present, DFSClient stores reference to configuration object. Since, these configuration objects are pretty big at times can blot the processes which has multiple DFSClient objects like in TaskTracker. This is an attempt to remove the reference of conf object in DFSClient.

      This patch creates a light inner conf class and copies the required keys from the Configuration object.

      1. HDFS-2092-2.patch
        16 kB
        Bharath Mundlapudi
      2. HDFS-2092-1.patch
        17 kB
        Bharath Mundlapudi

        Issue Links

          Activity

          Hide
          Bharath Mundlapudi added a comment -

          Attaching a patch for this

          Show
          Bharath Mundlapudi added a comment - Attaching a patch for this
          Hide
          Tsz Wo Nicholas Sze added a comment -

          +1 patch looks good.

          Show
          Tsz Wo Nicholas Sze added a comment - +1 patch looks good.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12483505/HDFS-2092-1.patch
          against trunk revision 1138262.

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. 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.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these core unit tests:
          org.apache.hadoop.hdfs.server.balancer.TestBalancer
          org.apache.hadoop.hdfs.TestDFSPermission

          +1 contrib tests. The patch passed contrib unit tests.

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/819//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/819//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/819//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12483505/HDFS-2092-1.patch against trunk revision 1138262. +1 @author. The patch does not contain any @author tags. -1 tests included. 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. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these core unit tests: org.apache.hadoop.hdfs.server.balancer.TestBalancer org.apache.hadoop.hdfs.TestDFSPermission +1 contrib tests. The patch passed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/819//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/819//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/819//console This message is automatically generated.
          Hide
          Bharath Mundlapudi added a comment -

          Attaching a patch which address the failed test.

          Show
          Bharath Mundlapudi added a comment - Attaching a patch which address the failed test.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12483560/HDFS-2092-2.patch
          against trunk revision 1138645.

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. 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.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/826//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/826//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/826//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12483560/HDFS-2092-2.patch against trunk revision 1138645. +1 @author. The patch does not contain any @author tags. -1 tests included. 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. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/826//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/826//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/826//console This message is automatically generated.
          Hide
          Bharath Mundlapudi added a comment -

          I have moved uMask back to DFSClient as it was before instead of creating at the construction of DFSClient.Conf. Typically, Conf should be created once and should be sent to FileSystem.get.

          Like,

          conf = new Configuration();
          ... //set conf related key, values here
          fs = FileSystem.get(conf); 
          

          But someone can do the following today:

          conf = new Configuration();
          ... //set conf related key, values here
          fs = FileSystem.get(conf); 
          //set somemore or change the conf key, values here.
          

          Clearly, this shouldn't be allowed. Today, conf is used as a context. It should be set once. If we want to change the conf then close the filesystem and re-init the conf.

          I will fix the uMask related changes in an another JIRA since this requires some refactoring.

          Show
          Bharath Mundlapudi added a comment - I have moved uMask back to DFSClient as it was before instead of creating at the construction of DFSClient.Conf. Typically, Conf should be created once and should be sent to FileSystem.get. Like, conf = new Configuration(); ... //set conf related key, values here fs = FileSystem.get(conf); But someone can do the following today: conf = new Configuration(); ... //set conf related key, values here fs = FileSystem.get(conf); //set somemore or change the conf key, values here. Clearly, this shouldn't be allowed. Today, conf is used as a context. It should be set once. If we want to change the conf then close the filesystem and re-init the conf. I will fix the uMask related changes in an another JIRA since this requires some refactoring.
          Hide
          Bharath Mundlapudi added a comment -

          Also, exiting unit tests should cover this path. So i haven't added new unit tests.

          Show
          Bharath Mundlapudi added a comment - Also, exiting unit tests should cover this path. So i haven't added new unit tests.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          I have committed this. Thanks, Bharath!

          Show
          Tsz Wo Nicholas Sze added a comment - I have committed this. Thanks, Bharath!
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #756 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/756/)
          HDFS-2092. Remove some object references to Configuration in DFSClient. Contributed by Bharath Mundlapudi

          szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1139097
          Files :

          • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/DFSOutputStream.java
          • /hadoop/common/trunk/hdfs/CHANGES.txt
          • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/DFSInputStream.java
          • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/DFSClient.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #756 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/756/ ) HDFS-2092 . Remove some object references to Configuration in DFSClient. Contributed by Bharath Mundlapudi szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1139097 Files : /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/DFSOutputStream.java /hadoop/common/trunk/hdfs/CHANGES.txt /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/DFSInputStream.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/DFSClient.java
          Hide
          Eli Collins added a comment -

          Does this change mean that a Configuration object can now bee free'd because there's one fewer ref to it? Otherwise it seems like we're now allocating a DFSClient#conf in addition to a Configuration object which increases over all memory usage no?

          Show
          Eli Collins added a comment - Does this change mean that a Configuration object can now bee free'd because there's one fewer ref to it? Otherwise it seems like we're now allocating a DFSClient#conf in addition to a Configuration object which increases over all memory usage no?
          Hide
          Bharath Mundlapudi added a comment -

          Hi Eli,

          >> Does this change mean that a Configuration object can now bee free'd because there's one fewer ref to it?
          Yes, the direction of this patch is that. Eventually, we will be passing around only the DFSClient#conf or only required parameters to the downstream. This will be a big change and needs border discussion. But you are right, the idea is to stop having references to the conf object coming from the users. We want to let client code to decide the scope of conf object.

          Regarding memory, these will be few [key,value] pairs copied into DFSClient but then will be freeing the blotted conf object for the GC. That will be a big win on memory.

          Show
          Bharath Mundlapudi added a comment - Hi Eli, >> Does this change mean that a Configuration object can now bee free'd because there's one fewer ref to it? Yes, the direction of this patch is that. Eventually, we will be passing around only the DFSClient#conf or only required parameters to the downstream. This will be a big change and needs border discussion. But you are right, the idea is to stop having references to the conf object coming from the users. We want to let client code to decide the scope of conf object. Regarding memory, these will be few [key,value] pairs copied into DFSClient but then will be freeing the blotted conf object for the GC. That will be a big win on memory.
          Hide
          Eli Collins added a comment -

          Makes sense, was assuming that was the context, just wanted to double check because I didn't see other jiras that would remove the other remaining refs.

          Out of curiosity, how big is your typical Configuration object? Ie how much memory will a typical client save once Configuration becomes a short-lived class?

          Show
          Eli Collins added a comment - Makes sense, was assuming that was the context, just wanted to double check because I didn't see other jiras that would remove the other remaining refs. Out of curiosity, how big is your typical Configuration object? Ie how much memory will a typical client save once Configuration becomes a short-lived class?
          Hide
          Bharath Mundlapudi added a comment -

          I am going to create umbrella jira for this. Link these Jira's to that soon. Thanks for asking.

          Regarding how much we save in memory is a context dependent meaning clients usage. From my past studies on this shows the following distribution.

          Cluster1:

          Quantile (Size in Bytes):
          1% 50% 90% 95% 99% 100%
          20958.0 27277.0 115043.4 150415.7 273566.7 82323144.0

          Cluster2:

          Quantile (Size in Bytes):
          1% 50% 90% 95% 99% 100%
          21087.0 46540.0 153724.0 240260.0 386204.8 7885728.0

          Show
          Bharath Mundlapudi added a comment - I am going to create umbrella jira for this. Link these Jira's to that soon. Thanks for asking. Regarding how much we save in memory is a context dependent meaning clients usage. From my past studies on this shows the following distribution. Cluster1: Quantile (Size in Bytes): 1% 50% 90% 95% 99% 100% 20958.0 27277.0 115043.4 150415.7 273566.7 82323144.0 Cluster2: Quantile (Size in Bytes): 1% 50% 90% 95% 99% 100% 21087.0 46540.0 153724.0 240260.0 386204.8 7885728.0
          Hide
          Aaron T. Myers added a comment -

          If I read that right, we're talking about a change that at the 99th percentile saves at most 386kb? I'm skeptical that those modest savings warrant this change.

          Also, how exactly were these gains measured? In what unit can we expect these memory savings? i.e. per TT? per DFSClient instance?

          Show
          Aaron T. Myers added a comment - If I read that right, we're talking about a change that at the 99th percentile saves at most 386kb? I'm skeptical that those modest savings warrant this change. Also, how exactly were these gains measured? In what unit can we expect these memory savings? i.e. per TT? per DFSClient instance?
          Hide
          Bharath Mundlapudi added a comment -

          Hi Aaron,

          That was just a sample of measurement for a day. We should care for MAX here in this case. Also, Going forward, PIG 0.9 will store lots of meta data in the conf and also one can embed the PIG script itself in the conf. This can potentially blow the TT. We can measure an approx size of conf by the job.xml file in the job history location. Since one can store anything in the job conf, we should be careful with the references to this object - we should not hold for long duration.

          Show
          Bharath Mundlapudi added a comment - Hi Aaron, That was just a sample of measurement for a day. We should care for MAX here in this case. Also, Going forward, PIG 0.9 will store lots of meta data in the conf and also one can embed the PIG script itself in the conf. This can potentially blow the TT. We can measure an approx size of conf by the job.xml file in the job history location. Since one can store anything in the job conf, we should be careful with the references to this object - we should not hold for long duration.
          Hide
          Aaron T. Myers added a comment -

          Note: I'm not necessarily opposed to this change, but please justify its usefulness. From what I can tell so far, this patch seems to be optimizing something that's not actually an issue.

          That was just a sample of measurement for a day.

          Sure, but what was it actually measuring? Increase in child heap size per task attempt? Increase in heap size per TT? Something else?

          Also, Going forward, PIG 0.9 will store lots of meta data in the conf and also one can embed the PIG script itself in the conf.

          I don't know much about Pig, but that sounds like a bad idea on its part. Maybe I'm wrong about that.

          This can potentially blow the TT.

          Can it? I've seen users have a lot of different problems with Hadoop, but Task Trackers falling over because of conf objects being too large isn't one I can recall.

          Since one can store anything in the job conf, we should be careful with the references to this object - we should not hold for long duration.

          At most these references will be held for the lifetime of a task attempt, right? So not so long?

          Show
          Aaron T. Myers added a comment - Note: I'm not necessarily opposed to this change, but please justify its usefulness. From what I can tell so far, this patch seems to be optimizing something that's not actually an issue. That was just a sample of measurement for a day. Sure, but what was it actually measuring? Increase in child heap size per task attempt? Increase in heap size per TT? Something else? Also, Going forward, PIG 0.9 will store lots of meta data in the conf and also one can embed the PIG script itself in the conf. I don't know much about Pig, but that sounds like a bad idea on its part. Maybe I'm wrong about that. This can potentially blow the TT. Can it? I've seen users have a lot of different problems with Hadoop, but Task Trackers falling over because of conf objects being too large isn't one I can recall. Since one can store anything in the job conf, we should be careful with the references to this object - we should not hold for long duration. At most these references will be held for the lifetime of a task attempt, right? So not so long?
          Hide
          Bharath Mundlapudi added a comment -

          We are not concerned about the task attempt. The problem here is for Task Tracker's availability. The way conf was designed has its own benefits. At the same time it comes with some disadvantages. What if a task attempt can run for a day or more? This is not uncommon in, our clusters.

          Again, I am listing couple of issues,
          1. With UGI, conf will be created per user in TT. (Security folks?)
          2. PIG or any other job can store arbitrary data. Hadoop framework should be able to deal with it as far as it can.
          3. Last but not least, API should not hold on to client's data.

          As every job is different so can workloads can be different. So one can't see or hear all the problems.

          Show
          Bharath Mundlapudi added a comment - We are not concerned about the task attempt. The problem here is for Task Tracker's availability. The way conf was designed has its own benefits. At the same time it comes with some disadvantages. What if a task attempt can run for a day or more? This is not uncommon in, our clusters. Again, I am listing couple of issues, 1. With UGI, conf will be created per user in TT. (Security folks?) 2. PIG or any other job can store arbitrary data. Hadoop framework should be able to deal with it as far as it can. 3. Last but not least, API should not hold on to client's data. As every job is different so can workloads can be different. So one can't see or hear all the problems.
          Hide
          Aaron T. Myers added a comment -

          We are not concerned about the task attempt. The problem here is for Task Tracker's availability.

          Have you actually experienced TTs crashing because conf objects were too large? Or where conf objects were taking up a substantial portion of the available heap space?

          The way conf was designed has its own benefits. At the same time it comes with some disadvantages. What if a task attempt can run for a day or more? This is not uncommon in, our clusters.

          I would conjecture that such a task attempt is likely using many MBs or GBs of memory for the actual work it's doing. Is this patch which saves a few hundred KBs at the extreme end really going to move the needle?

          1. With UGI, conf will be created per user in TT. (Security folks?)

          But presumably only for every user which is concurrently running a task attempt on that TT, so not that many, right? Unless I'm missing something, which is certainly possible.

          2. PIG or any other job can store arbitrary data. Hadoop framework should be able to deal with it as far as it can.

          No disagreement there.

          3. Last but not least, API should not hold on to client's data.

          I see no principled reason the DFSClient "should not hold on to client's data" in the form of the conf object. If this is actually negatively impacting performance or availability, then we should certainly fix that, but you haven't demonstrated that yet.

          As every job is different so can workloads can be different. So one can't see or hear all the problems.

          Certainly, but we can validate this issue with some testing. Can you please describe what you did to gather these measurements? What exactly are they actually measuring?

          My issue here is that this change is being done purely as an optimization, but it's unclear to me that negative issues exist without this patch, or that this patch necessarily addresses those issues. If you can demonstrate those, I'll shut up immediately.

          Show
          Aaron T. Myers added a comment - We are not concerned about the task attempt. The problem here is for Task Tracker's availability. Have you actually experienced TTs crashing because conf objects were too large? Or where conf objects were taking up a substantial portion of the available heap space? The way conf was designed has its own benefits. At the same time it comes with some disadvantages. What if a task attempt can run for a day or more? This is not uncommon in, our clusters. I would conjecture that such a task attempt is likely using many MBs or GBs of memory for the actual work it's doing. Is this patch which saves a few hundred KBs at the extreme end really going to move the needle? 1. With UGI, conf will be created per user in TT. (Security folks?) But presumably only for every user which is concurrently running a task attempt on that TT, so not that many, right? Unless I'm missing something, which is certainly possible. 2. PIG or any other job can store arbitrary data. Hadoop framework should be able to deal with it as far as it can. No disagreement there. 3. Last but not least, API should not hold on to client's data. I see no principled reason the DFSClient "should not hold on to client's data" in the form of the conf object. If this is actually negatively impacting performance or availability, then we should certainly fix that, but you haven't demonstrated that yet. As every job is different so can workloads can be different. So one can't see or hear all the problems. Certainly, but we can validate this issue with some testing. Can you please describe what you did to gather these measurements? What exactly are they actually measuring? My issue here is that this change is being done purely as an optimization, but it's unclear to me that negative issues exist without this patch, or that this patch necessarily addresses those issues. If you can demonstrate those, I'll shut up immediately.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #705 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/705/)
          HDFS-2092. Remove some object references to Configuration in DFSClient. Contributed by Bharath Mundlapudi

          szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1139097
          Files :

          • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/DFSOutputStream.java
          • /hadoop/common/trunk/hdfs/CHANGES.txt
          • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/DFSInputStream.java
          • /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/DFSClient.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #705 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/705/ ) HDFS-2092 . Remove some object references to Configuration in DFSClient. Contributed by Bharath Mundlapudi szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1139097 Files : /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/DFSOutputStream.java /hadoop/common/trunk/hdfs/CHANGES.txt /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/DFSInputStream.java /hadoop/common/trunk/hdfs/src/java/org/apache/hadoop/hdfs/DFSClient.java
          Hide
          Tsz Wo Nicholas Sze added a comment -

          Hi Aaron, we did see some cases in the past that some users put a large object in conf and then JT/TT ran out of memory. Indeed, users can put arbitrary large objects in conf. So this change also prevents such problems.

          Show
          Tsz Wo Nicholas Sze added a comment - Hi Aaron, we did see some cases in the past that some users put a large object in conf and then JT/TT ran out of memory. Indeed, users can put arbitrary large objects in conf. So this change also prevents such problems.
          Hide
          Aaron T. Myers added a comment -

          Hi Aaron, we did see some cases in the past that some users put a large object in conf and then JT/TT ran out of memory. Indeed, users can put arbitrary large objects in conf.

          Thanks for this explanation, Nicholas. That does indeed seem like a problem worthy of attack.

          So this change also prevents such problems.

          I'm not entirely convinced of this. Does this change definitely prevent these problems? Is it really the case that the JT could've garbage collected these JobConf instances, were it not for the DFSClient still holding a reference? If that's the intended goal, I'd really like to see a little benchmark done demonstrating the memory use of the JT with large JobConf objects before and after this patch. If this patch does indeed address this issue, I could even imagine a unit test being written which could ensure that no long-lived JobConf references sneak back into the JT.

          Show
          Aaron T. Myers added a comment - Hi Aaron, we did see some cases in the past that some users put a large object in conf and then JT/TT ran out of memory. Indeed, users can put arbitrary large objects in conf. Thanks for this explanation, Nicholas. That does indeed seem like a problem worthy of attack. So this change also prevents such problems. I'm not entirely convinced of this. Does this change definitely prevent these problems? Is it really the case that the JT could've garbage collected these JobConf instances, were it not for the DFSClient still holding a reference? If that's the intended goal, I'd really like to see a little benchmark done demonstrating the memory use of the JT with large JobConf objects before and after this patch. If this patch does indeed address this issue, I could even imagine a unit test being written which could ensure that no long-lived JobConf references sneak back into the JT.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          We are fixing DFSClient but not JT. Any system which indirectly uses DFSClient has this problem.

          Show
          Tsz Wo Nicholas Sze added a comment - We are fixing DFSClient but not JT. Any system which indirectly uses DFSClient has this problem.
          Hide
          Todd Lipcon added a comment -

          I see a couple of good reasons for the change:

          • changing a conf object underneath a DFSClient won't change its behavior
          • we no longer have to worry about that deadlock bug of synchronized (conf) { conf.writeToXml(dfsOutputStream); }

          But the memory leak argument seems strange. For TIP/JIP, those classes already keep a reference to JobConf. So eliminating this reference to the object shouldn't have any effect on memory usage, right? Or is the issue that we're leaking FileSystem objects into the FS Cache in the TT/JT? In that case, we ought to fix that leak too.

          Show
          Todd Lipcon added a comment - I see a couple of good reasons for the change: changing a conf object underneath a DFSClient won't change its behavior we no longer have to worry about that deadlock bug of synchronized (conf) { conf.writeToXml(dfsOutputStream); } But the memory leak argument seems strange. For TIP/JIP, those classes already keep a reference to JobConf. So eliminating this reference to the object shouldn't have any effect on memory usage, right? Or is the issue that we're leaking FileSystem objects into the FS Cache in the TT/JT? In that case, we ought to fix that leak too.
          Hide
          Bharath Mundlapudi added a comment -

          Todd, Thanks for the reasons.

          When we mean a client it can be anything, like TT/JT which has TIP/JIP. You are right, client TIP/JIP can have references to JobConf. But then reference scope is decided by client. And yes, eventually, we need to fix the FS cache you are referring also if there are any leaks.

          Show
          Bharath Mundlapudi added a comment - Todd, Thanks for the reasons. When we mean a client it can be anything, like TT/JT which has TIP/JIP. You are right, client TIP/JIP can have references to JobConf. But then reference scope is decided by client. And yes, eventually, we need to fix the FS cache you are referring also if there are any leaks.

            People

            • Assignee:
              Bharath Mundlapudi
              Reporter:
              Bharath Mundlapudi
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development