Hadoop Map/Reduce
  1. Hadoop Map/Reduce
  2. MAPREDUCE-1938

Ability for having user's classes take precedence over the system classes for tasks' classpath

    Details

      Description

      It would be nice to have the ability in MapReduce to allow users to specify for their jobs alternate implementations of classes that are already defined in the MapReduce libraries. For example, an alternate implementation for CombineFileInputFormat.

      1. mr-1938-bp20.patch
        5 kB
        Devaraj Das
      2. mr-1938-bp20.1.patch
        6 kB
        Devaraj Das
      3. mapred-1938-1
        14 kB
        Krishna Ramachandran
      4. mapred-1938-2.patch
        7 kB
        Krishna Ramachandran
      5. mapred-1938-3.patch
        8 kB
        Krishna Ramachandran

        Issue Links

          Activity

          Hide
          Devaraj Das added a comment -

          Patch for Y20. Not for commit here.

          The patch defines a configuration option using which users can specify that the system classpath be added last in the list of classpaths created before launching a task. It also introduces a HADOOP_CLIENT_CLASSPATH env variable in the hadoop shell script that should be set to point to the user's jar/classpath (containing his implementation of the system classes) before submitting a job over command line.

          Show
          Devaraj Das added a comment - Patch for Y20. Not for commit here. The patch defines a configuration option using which users can specify that the system classpath be added last in the list of classpaths created before launching a task. It also introduces a HADOOP_CLIENT_CLASSPATH env variable in the hadoop shell script that should be set to point to the user's jar/classpath (containing his implementation of the system classes) before submitting a job over command line.
          Hide
          Doug Cutting added a comment -

          Two thoughts:
          1. In general, we need to better separate the kernel from the library. CombineFileInputFormat is library code and should be easy to update without updating the cluster. Long-term, only kernel code should be hardwired on the classpath of tasks, with library and user code both specified per job. There should be no default version of library classes for a task: tasks should always specify their required libraries. Is there a Jira for this? I know Tom's expressed interest in working on this.
          2. We should permit user code to depend on different versions of things than the kernel does. For example, user code might rely on a different version of HttpClient or Avro than that used by MapReduce. This should be possible if instances of classes from these are not a passed between user and kernel code, e.g., as long as Avro and HttpClient classes are not a part of the MapReduce API. In this case classloaders (probably via OSGI) could permit this.

          Show
          Doug Cutting added a comment - Two thoughts: 1. In general, we need to better separate the kernel from the library. CombineFileInputFormat is library code and should be easy to update without updating the cluster. Long-term, only kernel code should be hardwired on the classpath of tasks, with library and user code both specified per job. There should be no default version of library classes for a task: tasks should always specify their required libraries. Is there a Jira for this? I know Tom's expressed interest in working on this. 2. We should permit user code to depend on different versions of things than the kernel does. For example, user code might rely on a different version of HttpClient or Avro than that used by MapReduce. This should be possible if instances of classes from these are not a passed between user and kernel code, e.g., as long as Avro and HttpClient classes are not a part of the MapReduce API. In this case classloaders (probably via OSGI) could permit this.
          Hide
          Owen O'Malley added a comment -

          I think that the default for this should be on.

          Rather than add HADOOP_CLIENT_CLASSPATH, let's make a new variable HADOOP_USER_CLASSPATH_LAST. If it is defined, we add HADOOP_CLASSPATH to the tail like we currently do. Otherwise it is added to the front.

          Show
          Owen O'Malley added a comment - I think that the default for this should be on. Rather than add HADOOP_CLIENT_CLASSPATH, let's make a new variable HADOOP_USER_CLASSPATH_LAST. If it is defined, we add HADOOP_CLASSPATH to the tail like we currently do. Otherwise it is added to the front.
          Hide
          Owen O'Malley added a comment -

          Doug,

          I agree that the kernel code should be split out from libraries, however, that work is much more involved. I don't see a problem with putting the user's code first. It is not a security concern. The user's code is only run as the user. Furthermore, it doesn't actually stop them from loading system classes. They can exec a new jvm with a new class path of their own choosing.

          Therefore, by putting the user's classes last all that we've done is make it harder for the user to implement hot fixes in their own jobs. That doesn't seem like a good goal.

          Show
          Owen O'Malley added a comment - Doug, I agree that the kernel code should be split out from libraries, however, that work is much more involved. I don't see a problem with putting the user's code first. It is not a security concern. The user's code is only run as the user. Furthermore, it doesn't actually stop them from loading system classes. They can exec a new jvm with a new class path of their own choosing. Therefore, by putting the user's classes last all that we've done is make it harder for the user to implement hot fixes in their own jobs. That doesn't seem like a good goal.
          Hide
          Doug Cutting added a comment -

          Owen, I agree with your analysis. I'm just trying to put this patch in context of these other related discussions.

          This patch addresses some issues relevant to separation of kernel & library. In common cases one can merely provide an alternate version of the library class in one's job. Fully separating kernel & library with a well-defined, minimal kernel API is clearly aesthetically better. Are there use cases that will that enable that this patch will not? I think mostly it will just make it clear which classes are safe to replace with updated versions and which are not. Does that sound right?

          The issue of user versions of libraries that the kernel uses (like Avro, log4j, HttpClient, etc.) is not entirely addressed by this patch. If the user's version is backwards compatible with the kernel's version then this patch is sufficient. But if the user's version of a library makes incompatible changes then we'd need a classloader/OSGI solution. Even then, I think it only works if user and kernel code do not interchange instances of classes defined by these libraries. A minimal kernel API will help reduce that risk. Does this analysis sound right?

          I'm trying to understand how far this patch gets us towards those goals: what it solves and what it doesn't.

          Show
          Doug Cutting added a comment - Owen, I agree with your analysis. I'm just trying to put this patch in context of these other related discussions. This patch addresses some issues relevant to separation of kernel & library. In common cases one can merely provide an alternate version of the library class in one's job. Fully separating kernel & library with a well-defined, minimal kernel API is clearly aesthetically better. Are there use cases that will that enable that this patch will not? I think mostly it will just make it clear which classes are safe to replace with updated versions and which are not. Does that sound right? The issue of user versions of libraries that the kernel uses (like Avro, log4j, HttpClient, etc.) is not entirely addressed by this patch. If the user's version is backwards compatible with the kernel's version then this patch is sufficient. But if the user's version of a library makes incompatible changes then we'd need a classloader/OSGI solution. Even then, I think it only works if user and kernel code do not interchange instances of classes defined by these libraries. A minimal kernel API will help reduce that risk. Does this analysis sound right? I'm trying to understand how far this patch gets us towards those goals: what it solves and what it doesn't.
          Hide
          Devaraj Das added a comment -

          Addressing Owen's comment on the shell script part of the patch.

          Doug, this patch is a first step towards letting users use their own versions of library provided implementation for things like CombineFileInputFormat. The use case is to allow for specific implementations of library classes for certain classes of jobs.

          This doesn't aim to address the kernel/library separation in its entirety. So yes, if the user puts a class on the classpath that doesn't work with the kernel compatibly then tasks will fail, or produce obscure/inconsistent results, but that will affect only that job, and the user would notice that soon (hopefully). Did i understand your concern right?

          Show
          Devaraj Das added a comment - Addressing Owen's comment on the shell script part of the patch. Doug, this patch is a first step towards letting users use their own versions of library provided implementation for things like CombineFileInputFormat. The use case is to allow for specific implementations of library classes for certain classes of jobs. This doesn't aim to address the kernel/library separation in its entirety. So yes, if the user puts a class on the classpath that doesn't work with the kernel compatibly then tasks will fail, or produce obscure/inconsistent results, but that will affect only that job, and the user would notice that soon (hopefully). Did i understand your concern right?
          Hide
          Doug Cutting added a comment -

          > Did i understand your concern right?

          I don't have specific concerns about this patch. Sorry for any confusion in that regard. I thought it worthwhile to discuss how this change relates to other changes that are contemplated. It seems not inconsistent, provides some of the benefits, and is considerably simpler; in short, a good thing.

          Show
          Doug Cutting added a comment - > Did i understand your concern right? I don't have specific concerns about this patch. Sorry for any confusion in that regard. I thought it worthwhile to discuss how this change relates to other changes that are contemplated. It seems not inconsistent, provides some of the benefits, and is considerably simpler; in short, a good thing.
          Hide
          Owen O'Malley added a comment -

          This patch basically puts the user in charge of their job. They can leave the safety switch set in which case they get the current behavior. But if they turn off the safety, their classes go ahead of the ones installed on the cluster. That means that they can break things, but all they can break is their own tasks.

          After we do the split of core from library, you still need this switch. There will always be the possibility of needing to patch something in the core, because even MapTask has bugs. smile After splitting them apart, we can put the library code at the very end

          safety on: core, user, library
          safety off: user, core, library

          This patch is just about providing the safety switch.

          Show
          Owen O'Malley added a comment - This patch basically puts the user in charge of their job. They can leave the safety switch set in which case they get the current behavior. But if they turn off the safety, their classes go ahead of the ones installed on the cluster. That means that they can break things, but all they can break is their own tasks. After we do the split of core from library, you still need this switch. There will always be the possibility of needing to patch something in the core, because even MapTask has bugs. smile After splitting them apart, we can put the library code at the very end safety on: core, user, library safety off: user, core, library This patch is just about providing the safety switch.
          Hide
          David Rosenstrauch added a comment -

          Just wondering: does anyone know if Cloudera has plans to include this patch in the CDH any time soon?

          Show
          David Rosenstrauch added a comment - Just wondering: does anyone know if Cloudera has plans to include this patch in the CDH any time soon?
          Hide
          Krishna Ramachandran added a comment -

          We were going thro' major outstanding issues in the trunk
          Thought will upload a preliminary port of Devaraj's patch to trunk + a simple test case

          Show
          Krishna Ramachandran added a comment - We were going thro' major outstanding issues in the trunk Thought will upload a preliminary port of Devaraj's patch to trunk + a simple test case
          Hide
          Luke Lu added a comment -

          A couple of questions regarding the patch (filename should probably end with .patch):

          1. Are the new property a system/cluster property or a per job property? i.e, allowing user to to override the behavior (default to system jars first) per job. Owen's comments seem to imply it's a per job switch, which should be defined/documented in MRJobConfig (MAPREDUCE-1749)
          2. Why there are two boolean properties defined in the trunk patch "mapreduce.user.classpath.first" (defined in TaskRunner) and "mapreduce.task.classpath.user.precedence" (defined in JobContext)? The latter seems to have no effect and not tested.
          3. Why use Vector (in the test code) when there doesn't seem to be need for the list to be synchronized?
          4. Why do the test code use hard code paths and string literals when there are corresponding constants defined?
          Show
          Luke Lu added a comment - A couple of questions regarding the patch (filename should probably end with .patch): Are the new property a system/cluster property or a per job property? i.e, allowing user to to override the behavior (default to system jars first) per job. Owen's comments seem to imply it's a per job switch, which should be defined/documented in MRJobConfig ( MAPREDUCE-1749 ) Why there are two boolean properties defined in the trunk patch "mapreduce.user.classpath.first" (defined in TaskRunner) and "mapreduce.task.classpath.user.precedence" (defined in JobContext)? The latter seems to have no effect and not tested. Why use Vector (in the test code) when there doesn't seem to be need for the list to be synchronized? Why do the test code use hard code paths and string literals when there are corresponding constants defined?
          Hide
          Krishna Ramachandran added a comment -

          Luke
          Last one was a preliminary/quick port of what was there and lot of changes were redundant. I have cleaned it up in this rev.

          As for moving to MRJobConfig, based on discussions, my understanding is, desired to have this flag semi-private (MRJobConfig i believe is very public)

          Show
          Krishna Ramachandran added a comment - Luke Last one was a preliminary/quick port of what was there and lot of changes were redundant. I have cleaned it up in this rev. As for moving to MRJobConfig, based on discussions, my understanding is, desired to have this flag semi-private (MRJobConfig i believe is very public)
          Hide
          Luke Lu added a comment -

          Nits in the test code aside, +1

          Show
          Luke Lu added a comment - Nits in the test code aside, +1
          Hide
          Krishna Ramachandran added a comment -

          testpatch output

          [exec] -1 overall.
          [exec]
          [exec] +1 @author. The patch does not contain any @author tags.
          [exec]
          [exec] +1 tests included. The patch appears to include 5 new or modified tests.
          [exec]
          [exec] +1 javadoc. The javadoc tool did not generate any warning messages.
          [exec]
          [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings.
          [exec]
          [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings.
          [exec]
          [exec] -1 Eclipse classpath. The patch causes the Eclipse classpath to differ from the contents of the lib directories.

          Show
          Krishna Ramachandran added a comment - testpatch output [exec] -1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 5 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] -1 Eclipse classpath. The patch causes the Eclipse classpath to differ from the contents of the lib directories.
          Hide
          Amareshwari Sriramadasu added a comment -

          As for moving to MRJobConfig, based on discussions, my understanding is, desired to have this flag semi-private (MRJobConfig i believe is very public).

          MRJobConfig is the place to hold all job-level config paramaters. And it is not public. Annotation says "private, evolving". So, the configuration should be in MRJobConfig. Also, the name should say mapreduce.job.user.classpath.first saying it is a job level property.
          Also, in your next patch, can you do the changes in test-case suggested by Luke?

          Are you planning to raise a Hadoop-common jira for the changes in bin/hadoop script?

          Show
          Amareshwari Sriramadasu added a comment - As for moving to MRJobConfig, based on discussions, my understanding is, desired to have this flag semi-private (MRJobConfig i believe is very public). MRJobConfig is the place to hold all job-level config paramaters. And it is not public. Annotation says "private, evolving". So, the configuration should be in MRJobConfig. Also, the name should say mapreduce.job.user.classpath.first saying it is a job level property. Also, in your next patch, can you do the changes in test-case suggested by Luke? Are you planning to raise a Hadoop-common jira for the changes in bin/hadoop script?
          Hide
          Krishna Ramachandran added a comment -

          revised per previous comment
          + Luke's test case review - vector is API requirement - can't modify

          Yes there will be a separate jira for bin/hadoop client command line changes

          Show
          Krishna Ramachandran added a comment - revised per previous comment + Luke's test case review - vector is API requirement - can't modify Yes there will be a separate jira for bin/hadoop client command line changes
          Hide
          Alejandro Abdelnur added a comment -

          IMO this is not the right way of approaching this issue as it will lead to have multiple implementations of a library in the classpath and that sooner or later breaks things in very odd ways. Following an example of an issue I've run into few years ago:

          a-1.jar contains classes: A, B (with B using A)
          a-2.jar contains classes: A, C (with C using A)

          A classes from a-1.jar and a-2.jar are different.
          Kernel 'sometimes' uses class B from a-1.jar
          user-app 'sometimes' uses class C from a-2.jar

          All this will compile, but will create very odd errors at runtime.

          Because of this, I wouldn't provide a mechanism to allow users to do this.

          Instead, I think we should implement a task-classloader that only exposes to the application the MR API and the user is complete control of all the JARs in the app classpath, including input/output formats, etc, etc.

          I understand this can be a bit tricky because the line of API vs implementation classes is not that clear as in the servlet API (to bring an example of APIs with implementations doing this).

          Show
          Alejandro Abdelnur added a comment - IMO this is not the right way of approaching this issue as it will lead to have multiple implementations of a library in the classpath and that sooner or later breaks things in very odd ways. Following an example of an issue I've run into few years ago: a-1.jar contains classes: A, B (with B using A) a-2.jar contains classes: A, C (with C using A) A classes from a-1.jar and a-2.jar are different. Kernel 'sometimes' uses class B from a-1.jar user-app 'sometimes' uses class C from a-2.jar All this will compile, but will create very odd errors at runtime. Because of this, I wouldn't provide a mechanism to allow users to do this. Instead, I think we should implement a task-classloader that only exposes to the application the MR API and the user is complete control of all the JARs in the app classpath, including input/output formats, etc, etc. I understand this can be a bit tricky because the line of API vs implementation classes is not that clear as in the servlet API (to bring an example of APIs with implementations doing this).
          Hide
          Mahadev konar added a comment -

          alejandro, good point. I think the premise here was to provide a simple way without making too many changes, to allow users to have there libraries in the classpath. Also, I think the assumption is that the task code doesnt load that many libraries.I would think its more like a hack than a perfect solution but I'd be happy to see a solution like the one you proposed. Are you working on such a soln?

          Show
          Mahadev konar added a comment - alejandro, good point. I think the premise here was to provide a simple way without making too many changes, to allow users to have there libraries in the classpath. Also, I think the assumption is that the task code doesnt load that many libraries.I would think its more like a hack than a perfect solution but I'd be happy to see a solution like the one you proposed. Are you working on such a soln?
          Hide
          Eli Collins added a comment -

          This is a pretty serious bug that has ramifications for other jiras, let's make it a blocker for 22.

          Show
          Eli Collins added a comment - This is a pretty serious bug that has ramifications for other jiras, let's make it a blocker for 22.
          Hide
          Owen O'Malley added a comment -

          Alejandro, you are viewing this wrong. Of course the user shouldn't deliberately make conflicts without understanding the problem. This is about providing control to the user so that they can upgrade (or hotfix) a jar file for their job without installing it on the cluster.

          I would suggest that the default should be user jars and then the framework jars with a compatibility switch for backwards compatibility.

          Show
          Owen O'Malley added a comment - Alejandro, you are viewing this wrong. Of course the user shouldn't deliberately make conflicts without understanding the problem. This is about providing control to the user so that they can upgrade (or hotfix) a jar file for their job without installing it on the cluster. I would suggest that the default should be user jars and then the framework jars with a compatibility switch for backwards compatibility.
          Hide
          Alejandro Abdelnur added a comment -

          Mahadev,

          I'm not working on a patch for this at the moment. I wouldn't mind (I'll have to allocate some time for it), but before doing so, I need a clear cut on the APIs that are interface, and they should be in their own JAR file, separate from the JAR file with the implementation classes.

          Regarding your comment on 'task code does not load that many libraries', I'm not sure is a valid statement if you are doing Pig or Hive, just check how many more JARs they bring into the classpath, and then check of those JARs how many of them overlap with Hadoop JARs, and then check their versions.

          Owen,

          Users get in trouble without making conflicts in a deliberate way, they get into those conflicts because of the lack of isolation of the current model (refer to second paragraph of answer to Mahadev).

          Having the use in control as the default is, IMO, the wrong way to go, all the testing you've done on a release becomes meaningless as the users classpath will override any library used by the cluster by default.

          My take is that we should fix this via classloader isolation from the get go.

          If folks consider this to be a TOO big task (IMO, it will aways be) and the user&system classpath swap is a stop gap alternative, my take is that we should for a default of system-user classpath.

          I personally see this as a big issue (using Hadoop from Oozie means that I have to deal with libraries from Hadoop/Pig/Hive/Sqoop/Hbase in the classpath).

          Show
          Alejandro Abdelnur added a comment - Mahadev, I'm not working on a patch for this at the moment. I wouldn't mind (I'll have to allocate some time for it), but before doing so, I need a clear cut on the APIs that are interface, and they should be in their own JAR file, separate from the JAR file with the implementation classes. Regarding your comment on 'task code does not load that many libraries', I'm not sure is a valid statement if you are doing Pig or Hive, just check how many more JARs they bring into the classpath, and then check of those JARs how many of them overlap with Hadoop JARs, and then check their versions. Owen, Users get in trouble without making conflicts in a deliberate way, they get into those conflicts because of the lack of isolation of the current model (refer to second paragraph of answer to Mahadev). Having the use in control as the default is, IMO, the wrong way to go, all the testing you've done on a release becomes meaningless as the users classpath will override any library used by the cluster by default. My take is that we should fix this via classloader isolation from the get go. If folks consider this to be a TOO big task (IMO, it will aways be) and the user&system classpath swap is a stop gap alternative, my take is that we should for a default of system-user classpath. I personally see this as a big issue (using Hadoop from Oozie means that I have to deal with libraries from Hadoop/Pig/Hive/Sqoop/Hbase in the classpath).
          Hide
          Mahadev konar added a comment -

          alejandro, I think your claim might be a little broken in seeing this as a big overhaul of the class loading. Users I have mostly interacted with usually dont do so. Other users (advanced) usually want this to try things out or in case of urgent fixes (not already installed) want to override that with a new jar ( the new jar has all the dependencies it needs). I have usually seens users wanting to override one of the jars in the classpath. If you think otherwise, please do make changes as you see fit.

          Show
          Mahadev konar added a comment - alejandro, I think your claim might be a little broken in seeing this as a big overhaul of the class loading. Users I have mostly interacted with usually dont do so. Other users (advanced) usually want this to try things out or in case of urgent fixes (not already installed) want to override that with a new jar ( the new jar has all the dependencies it needs). I have usually seens users wanting to override one of the jars in the classpath. If you think otherwise, please do make changes as you see fit.
          Hide
          Tom White added a comment -

          BTW the classloader isolation issue is MAPREDUCE-1700.

          Show
          Tom White added a comment - BTW the classloader isolation issue is MAPREDUCE-1700 .
          Hide
          David Rosenstrauch added a comment -

          As per Mahadev's comment ("I have usually seens users wanting to override one of the jars in the classpath.") That's my use case and, I'd think, the major use case re: this bug.

          To clarify my issue: the version of Jackson that ships with Hadoop is old. But Avro, which I use in several of my M/R jobs, requires a newer version. Even if I supply the newer Jackson jar using -libjars, the job still picks up the old one (which has priority on the classpath) and so my job fails.

          There is no easy workaround for this. What we wound up doing to fix this issue in our cluster was to go into every single node, rename the jackson.jar out of the way, and then restart the daemons. Very manual and error prone process. (Not to mention a very inelegant fix.)

          So what's needed here is some way to tell Hadoop (without too much muss and fuss): for execution of this M/R job only, my version of the jackson.jar takes precedence over the one that comes shipped.

          Show
          David Rosenstrauch added a comment - As per Mahadev's comment ("I have usually seens users wanting to override one of the jars in the classpath.") That's my use case and, I'd think, the major use case re: this bug. To clarify my issue: the version of Jackson that ships with Hadoop is old. But Avro, which I use in several of my M/R jobs, requires a newer version. Even if I supply the newer Jackson jar using -libjars, the job still picks up the old one (which has priority on the classpath) and so my job fails. There is no easy workaround for this. What we wound up doing to fix this issue in our cluster was to go into every single node, rename the jackson.jar out of the way, and then restart the daemons. Very manual and error prone process. (Not to mention a very inelegant fix.) So what's needed here is some way to tell Hadoop (without too much muss and fuss): for execution of this M/R job only, my version of the jackson.jar takes precedence over the one that comes shipped.
          Hide
          Alejandro Abdelnur added a comment -

          A lot of testing goes into Hadoop before doing a release. All this testing is done using a well defined set of libraries and sometimes a particular version is used because of a bug fix or because the absence of a bug.

          By allowing users to override the libraries used by Hadoop (in the tasks) we are throwing overboard all the testing/certification done as part of a release as now pieces of the cluster (the tasks) will run with who-knows-what version of the different libraries.

          IMO, this feature will be the nightmare of the team in charge of operations/support of the Hadoop cluster.

          If we introduce this feature, we should have a switch at cluster config level that turns it off so admins can switch it off. And by default it should be off.

          Show
          Alejandro Abdelnur added a comment - A lot of testing goes into Hadoop before doing a release. All this testing is done using a well defined set of libraries and sometimes a particular version is used because of a bug fix or because the absence of a bug. By allowing users to override the libraries used by Hadoop (in the tasks) we are throwing overboard all the testing/certification done as part of a release as now pieces of the cluster (the tasks) will run with who-knows-what version of the different libraries. IMO, this feature will be the nightmare of the team in charge of operations/support of the Hadoop cluster. If we introduce this feature, we should have a switch at cluster config level that turns it off so admins can switch it off. And by default it should be off.
          Hide
          Owen O'Malley added a comment -

          If we introduce this feature, we should have a switch at cluster config level that turns it off so admins can switch it off. And by default it should be off.

          This feature just makes it easier. It is already possible. The cluster operator can force the attribute in the configuration making it more difficult again.

          Show
          Owen O'Malley added a comment - If we introduce this feature, we should have a switch at cluster config level that turns it off so admins can switch it off. And by default it should be off. This feature just makes it easier. It is already possible. The cluster operator can force the attribute in the configuration making it more difficult again.
          Hide
          Nigel Daley added a comment -

          Too late for 0.22. Moving Fix Version from 0.22 to 0.23.

          Show
          Nigel Daley added a comment - Too late for 0.22. Moving Fix Version from 0.22 to 0.23.
          Hide
          Scott Carey added a comment -

          Users get in trouble without making conflicts in a deliberate way, they get into those conflicts because of the lack of isolation of the current model (refer to second paragraph of answer to Mahadev).

          Having the use in control as the default is, IMO, the wrong way to go, all the testing you've done on a release becomes meaningless as the users classpath will override any library used by the cluster by default.

          My take is that we should fix this via classloader isolation from the get go.

          I agree and disagree. If someone volunteered and fixed the whole situation with classloader and/or shade and/or OSGi then great, this would not be needed. However, at this point I'd rather be given a crude tool with varioius risks than no tool at all. No cluster since 0.18 has worked for me without classpath collisions requiring manual replacement of libraries.

          So all that precious testing on the release you mention is useless to me since such a cluster can't run my jobs!

          Too late for 0.22. Moving Fix Version from 0.22 to 0.23.

          Considering this is in the Y! disro (and perhaps CDH3 ?) is there actually any veto for getting this in 0.22? Its simplistic and does provide the user with a gun to shoot themselves in the foot, but the user interested in this already has a different gun, thats just harder to use and affects all tasks in the cluster instead of selected ones.

          Show
          Scott Carey added a comment - Users get in trouble without making conflicts in a deliberate way, they get into those conflicts because of the lack of isolation of the current model (refer to second paragraph of answer to Mahadev). Having the use in control as the default is, IMO, the wrong way to go, all the testing you've done on a release becomes meaningless as the users classpath will override any library used by the cluster by default. My take is that we should fix this via classloader isolation from the get go. I agree and disagree. If someone volunteered and fixed the whole situation with classloader and/or shade and/or OSGi then great, this would not be needed. However, at this point I'd rather be given a crude tool with varioius risks than no tool at all. No cluster since 0.18 has worked for me without classpath collisions requiring manual replacement of libraries. So all that precious testing on the release you mention is useless to me since such a cluster can't run my jobs! Too late for 0.22. Moving Fix Version from 0.22 to 0.23. Considering this is in the Y! disro (and perhaps CDH3 ?) is there actually any veto for getting this in 0.22? Its simplistic and does provide the user with a gun to shoot themselves in the foot, but the user interested in this already has a different gun, thats just harder to use and affects all tasks in the cluster instead of selected ones.
          Hide
          Arun C Murthy added a comment -

          Fixed in 0.20.2xx. Not relevant for MR2.

          Show
          Arun C Murthy added a comment - Fixed in 0.20.2xx. Not relevant for MR2.
          Hide
          Lars Francke added a comment -

          Is this documented somewhere? How would I find out about this possibility without having to use a search engine and stumbling across this and then having to read a patch or release notes?

          Honest question. I never know where, how and if things are being documented in Hadoop.

          Show
          Lars Francke added a comment - Is this documented somewhere? How would I find out about this possibility without having to use a search engine and stumbling across this and then having to read a patch or release notes? Honest question. I never know where, how and if things are being documented in Hadoop.
          Hide
          Harsh J added a comment -

          This isn't present in 0.23. Removing from Fix Version. I'll take Arun's word that it is not needed there either (though it'd look like API breakage, hrm?)

          It should be included in 0.22 though.

          Show
          Harsh J added a comment - This isn't present in 0.23. Removing from Fix Version. I'll take Arun's word that it is not needed there either (though it'd look like API breakage, hrm?) It should be included in 0.22 though.
          Hide
          Harsh J added a comment -

          Sorry, my bad. I meant to target another JIRA. Adding back the Fix Version.

          Show
          Harsh J added a comment - Sorry, my bad. I meant to target another JIRA. Adding back the Fix Version.
          Hide
          Harsh J added a comment -

          Not relevant for MR2.

          Apparently is still relevant for MR2:

          ./hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java utilizes it to setup proper classpath structures.

          If your comment meant some other form of relevancy, please do clarify. In any case, this is present as "mapreduce.job.user.classpath.first" in MR2

          Show
          Harsh J added a comment - Not relevant for MR2. Apparently is still relevant for MR2: ./hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/util/MRApps.java utilizes it to setup proper classpath structures. If your comment meant some other form of relevancy, please do clarify. In any case, this is present as "mapreduce.job.user.classpath.first" in MR2
          Hide
          Luke Lu added a comment -

          This is option is no longer needed after MAPREDUCE-1700 (except for very rare cases where you want to implement your own classloader).

          Show
          Luke Lu added a comment - This is option is no longer needed after MAPREDUCE-1700 (except for very rare cases where you want to implement your own classloader).
          Hide
          Harsh J added a comment -

          Thanks for clarifying Luke!

          Show
          Harsh J added a comment - Thanks for clarifying Luke!

            People

            • Assignee:
              Krishna Ramachandran
              Reporter:
              Devaraj Das
            • Votes:
              3 Vote for this issue
              Watchers:
              18 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development