From mapreduce-issues-return-1524-apmail-hadoop-mapreduce-issues-archive=hadoop.apache.org@hadoop.apache.org Tue Aug 04 04:33:32 2009 Return-Path: Delivered-To: apmail-hadoop-mapreduce-issues-archive@minotaur.apache.org Received: (qmail 67045 invoked from network); 4 Aug 2009 04:33:32 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.3) by minotaur.apache.org with SMTP; 4 Aug 2009 04:33:32 -0000 Received: (qmail 49874 invoked by uid 500); 4 Aug 2009 04:33:37 -0000 Delivered-To: apmail-hadoop-mapreduce-issues-archive@hadoop.apache.org Received: (qmail 49830 invoked by uid 500); 4 Aug 2009 04:33:37 -0000 Mailing-List: contact mapreduce-issues-help@hadoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: mapreduce-issues@hadoop.apache.org Delivered-To: mailing list mapreduce-issues@hadoop.apache.org Received: (qmail 49820 invoked by uid 99); 4 Aug 2009 04:33:37 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 04 Aug 2009 04:33:37 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=10.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.140] (HELO brutus.apache.org) (140.211.11.140) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 04 Aug 2009 04:33:35 +0000 Received: from brutus (localhost [127.0.0.1]) by brutus.apache.org (Postfix) with ESMTP id E537C234C045 for ; Mon, 3 Aug 2009 21:33:14 -0700 (PDT) Message-ID: <506199843.1249360394924.JavaMail.jira@brutus> Date: Mon, 3 Aug 2009 21:33:14 -0700 (PDT) From: "Vinod K V (JIRA)" To: mapreduce-issues@hadoop.apache.org Subject: [jira] Commented: (MAPREDUCE-476) extend DistributedCache to work locally (LocalJobRunner) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-JIRA-FingerPrint: 30527f35849b9dde25b450d4833f0394 X-Virus-Checked: Checked by ClamAV on apache.org [ https://issues.apache.org/jira/browse/MAPREDUCE-476?page=3Dcom.atlass= ian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=3D1= 2738811#action_12738811 ]=20 Vinod K V commented on MAPREDUCE-476: ------------------------------------- Replies in-line. bq. Is there anything blocking MAPREDUCE-711 that prevents it from being co= mmitted? No, I've asked Owen to commit it. bq. I had a very clever bug in there(caused by not thinking enough while re= solving a merge conflict) that deleted the current working directory, recur= sively, in one of the tests. (TaskRunner is hard-coded to delete current wo= rking directory, which is ok, since it's typically a child process; not ok = for LocalJobRunner.) Nice catch! I too had burnt myself once trying to refactor out the code in = Child.java to a thread, and ended up deleting my whole work space because o= f the same issue :( bq. The tricky bit is always fixing only the lines I've changed, and not al= l the lines in a given file, to preserve history and keep reviewing sane. That is correct. bq. This class should also have the variable number argument getLocalCache(= ) methods so that the corresponding methods in DistributedCache can be depr= ecated. Also, each method in DistributedCache should call the correponding = method in DistributedCacheManager class. bq. Don't think I agree here. We can deprecate the getLocalCache methods in= DistributedCache right away. They delegate to each other, and one of them = delegates to TrackerDistributedCacheManager. Ideally, I'd remove these alto= gether =E2=80=94 Hadoop internally does not use these methods with this pat= ch, and there's no sensible reason why someone else would, but since it's p= ublic, it's getting deprecated. But it's not being deprecated with a pointe= r to use something else; it's getting deprecated so that you don't use it a= t all. Okay, i thought those methods were used internally atleast. But if, as you'= ve said, they are not used at all internally, +1 for deprecating them all t= ogether. bq. Using .equals method at +150 if (cacheFile.type =3D=3D CacheFile.FileT= ype.ARCHIVE). If you feel strongly about this, happy to change it, but I th= ink =3D=3D is more consistent. I am fine, no problem with this. bq. TaskTracker.initialize() A new DistributedCacheManager is created every= time, so old files will not be deleted by the subsequent purgeCache. Eithe= r we should have only one cacheManager for a TaskTracker or DistributedCach= eManager.cachedArchives should be static. The same problem exists with the = deprecated purgeCache() method in DistributedCache.java bq. If my understanding is correct, in the course of normal operations, Tas= kTracker.initialize() is only called once, by TaskTracker.run(). At startup= time (and whenever the TaskTracker decides to lose all of its state and re= set itself, which is essentially the same), the TaskTracker initializes the= TrackerDistributedCacheManager object. This is also the only time it's all= owed to "clear" the cache, since there are for certain no tasks running at = initialization time that depend on those. Ah, I was under the impression that distribute cache files are purged EXPLI= CITLY when a TT reinitializes, but it looks like we do a full delete on Tas= kTracker's local files during re-init. So, the code in your patch will work= for now, but it won't be in future when we might need explicit purge of di= stributed cache files when they are owned by users themselves (HADOOP-4493)= . We will change it then, +1 for the current changes. bq. JobClient.java What happens to the code in here? Should this be refacto= red too/ are we going to do something about it? bq. Good question I do think that JobClient should be using some methods in= the filecache package, instead of hard-coding a lot of logic. That said, I= chose to stop somewhere to avoid making this patch even harder to review. = I think it's ripe for a future JIRA. Okay. bq. I think most of the getter/setter methods are deprecable and moveable t= o DistributedCacheManager. Or at least we should give some thought about it= . For most of them, I do see from the javadoc that they are only for intern= al use anyways. bq. I agree. A few of them are used to manage the Configuration object. (In= my mind, we're serializing and de-serializing a set of requirements for th= e distributed cache into the text configuration, and doing so a bit haphaza= rdly.) I was very tempted to remove all the ones that are only meant to be = internal, but Tom advised me that I need to keep them deprecated for a vers= ion. Again, I think moving those methods into a more private place is a goo= d task to do along with changing how JobClient calls into this stuff. +1. So are you planning to do in the next version or in this patch itself? bq. I don't use Pipes typically, and it doesn't seem to compile on my Mac. = I'll try it on a Linux machine, but if it's easy/handy for you, it'd be gre= at to verify that bug. Will do so. bq. TestDistributedCacheManager * Code related to setFileStamps in JobClien= t.java (+636 to +656) and testManagerFlow() (+71 to +74) can be refactored = into an internal-only method in DistributedCacheManager. bq. I extracted the method and moved it to TrackerDistributedCacheManager. = Now that that has "Tracker" in the name, it's a little misplaced, but it's = not too bad. I think that's agreeable for now. We can move it to a more appropriate plac= e in future if we find one. bq. Another point. We should consolidate TestMRWithDistributedCache, TestMi= niMRLocalFS and TestMiniMRDFSCaching. That's a lot of code in three differe= nt files testing mostly common code. May be another JIRA issue if you feel = so. bq. I'd prefer a separate JIRA. Fine. +1. > extend DistributedCache to work locally (LocalJobRunner) > -------------------------------------------------------- > > Key: MAPREDUCE-476 > URL: https://issues.apache.org/jira/browse/MAPREDUCE-476 > Project: Hadoop Map/Reduce > Issue Type: Improvement > Reporter: sam rash > Assignee: Philip Zeyliger > Priority: Minor > Attachments: HADOOP-2914-v1-full.patch, HADOOP-2914-v1-since-4041= .patch, HADOOP-2914-v2.patch, HADOOP-2914-v3.patch, MAPREDUCE-476-v2-vs-v3.= patch, MAPREDUCE-476-v2-vs-v3.try2.patch, MAPREDUCE-476-v2-vs-v4.txt, MAPRE= DUCE-476-v2.patch, MAPREDUCE-476-v3.patch, MAPREDUCE-476-v3.try2.patch, MAP= REDUCE-476-v4-requires-MR711.patch, MAPREDUCE-476.patch > > > The DistributedCache does not work locally when using the outlined recipe= at http://hadoop.apache.org/core/docs/r0.16.0/api/org/apache/hadoop/fileca= che/DistributedCache.html=20 > Ideally, LocalJobRunner would take care of populating the JobConf and cop= ying remote files to the local file sytem (http, assume hdfs =3D default fs= =3D local fs when doing local development. --=20 This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.