Return-Path: X-Original-To: apmail-hadoop-hdfs-dev-archive@minotaur.apache.org Delivered-To: apmail-hadoop-hdfs-dev-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 0FF17DF11 for ; Thu, 21 Feb 2013 00:29:09 +0000 (UTC) Received: (qmail 98146 invoked by uid 500); 21 Feb 2013 00:29:08 -0000 Delivered-To: apmail-hadoop-hdfs-dev-archive@hadoop.apache.org Received: (qmail 98060 invoked by uid 500); 21 Feb 2013 00:29:08 -0000 Mailing-List: contact hdfs-dev-help@hadoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: hdfs-dev@hadoop.apache.org Delivered-To: mailing list hdfs-dev@hadoop.apache.org Received: (qmail 98052 invoked by uid 99); 21 Feb 2013 00:29:08 -0000 Received: from minotaur.apache.org (HELO minotaur.apache.org) (140.211.11.9) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 21 Feb 2013 00:29:08 +0000 Received: from localhost (HELO mail-pa0-f42.google.com) (127.0.0.1) (smtp-auth username cdouglas, mechanism plain) by minotaur.apache.org (qpsmtpd/0.29) with ESMTP; Thu, 21 Feb 2013 00:29:07 +0000 Received: by mail-pa0-f42.google.com with SMTP id kq12so4340665pab.1 for ; Wed, 20 Feb 2013 16:29:07 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.68.240.33 with SMTP id vx1mr5390354pbc.173.1361406547110; Wed, 20 Feb 2013 16:29:07 -0800 (PST) Received: by 10.68.17.67 with HTTP; Wed, 20 Feb 2013 16:29:06 -0800 (PST) In-Reply-To: References: <1361390182.5901.YahooMailNeo@web125703.mail.ne1.yahoo.com> <1361401264.85659.YahooMailNeo@web125704.mail.ne1.yahoo.com> <1361401736.24828.YahooMailNeo@web125701.mail.ne1.yahoo.com> Date: Wed, 20 Feb 2013 16:29:06 -0800 Message-ID: Subject: Re: VOTE: HDFS-347 merge From: Chris Douglas To: hdfs-dev@hadoop.apache.org Content-Type: text/plain; charset=ISO-8859-1 The throughput on this thread is too high. Nicholas/Suresh: do either of you disagree with the general approach in HDFS-347? Holding an inevitable branch open causes a lot of pain (Suresh, you endured much of that personally with security, which had a lot of follow-on work). While it makes sense to block HDFS-347 from 2.x before all concerns are addressed, are most objections so fundamental that the merge to trunk should be prevented (except for HDFS-2246)? Is there related development that will be impacted? Colin won't disappear after this is committed, so if the answers to these questions are "no", then let's move forward. Todd: Yes, the idea that the project only allows optimizations that work on all platforms is idiotic, which is why nobody has suggested it. Given that HDFS-347 is a strictly better approach, once committed, there will be ample motivation to add support for other OSes and remove HDFS-2246 entirely. Nobody is confused about this. There's ample precedent for retaining obscure, clumsy features as a temporary stop-gap (e.g., service plugins, opaque blobs of bytes in Tasks, configurable combiner semantics). What's the virtue of insisting on removing this? Unless there was a lot of follow-on work, HDFS-2246 doesn't look like a lot of code... -C On Wed, Feb 20, 2013 at 3:40 PM, Todd Lipcon wrote: > On Wed, Feb 20, 2013 at 3:31 PM, Suresh Srinivas wrote: >>> Given that this is an optimization, and we have a ton of optimizations >>> which don't yet run on Windows, I don't think that should be >>> considered. Additionally, the Windows support has not yet been merged, >>> nor is it in any release, so this isn't a regression. >>> >> >> This is a critical functionality for HBase peformance and an optimization >> we consider >> very important to have. > > Too bad it doesn't work in any of the normal installations... none of > the packages for Hadoop would allow it to work, given that the data > directories will be owned by HDFS and not world readable, and > tasks/HBase would run as an "hbase" user, which wouldn't have direct > access to the block files. > >>> >>> I would be happy to review an addition to the HDFS-347 branch which >>> addresses this issue. But I don't think we should be maintaining two >>> codepaths for the sake of an optimization on a platform which is not >>> yet officially supported on trunk, especially when the old code path >>> is _insecure_ and thus unusable in most environments. >> >> >> I have to disagree. No where in the jira or the design it is explicitly >> stated that >> the old short circuit functionality is being removed. My assumption has been >> that it will not be removed. > > I've tried this avenue in the past on other insecurities which were > fixed. Sorry if you were depending on insecure behavior. The project > should move on and not have 3+ ways of implementing the same thing. > >> As regards "officially supported", we have been doing >> windows development for >> more than a year. In fact branch-1-win is being used by a lot users. Given >> merging it to branch-1 requires first making it available in trunk, we have >> been doing >> a lot of work in branch-trunk-win. It is almost ready to be merged as well. >> >> I am -1 on removing existing short circuit until an alternative short >> circuit similar >> to HDFS-347 on all the platforms. > > Great -- are you committed to building this equivalent feature for > Windows, then? On what timeline? From my viewpoint, Windows isn't a > supported platform *right now*, so vetoing based on it seems > meritless. > > BTW, the posix_fadvise based readahead is an important optimization > for many workloads, too, but from what I can tell looking at the > Windows branch, it doesn't support it. There are other places in the > Windows branch where performance is going to be worse - eg disabling > the pipelined crc32c implementation will be a 2-3x hit on that code > path. > > No one has voted to merge Windows support, and if merging Windows > support means that, from now on, every _optimization_ must work on > Windows, I don't think I will be able to vote +1. The vast majority of > the community is _not_ running Windows, and I don't want to block > progress on the small number of developers who know how to program > against that platform. > > If that's the axe hanging over our head with the Windows branch, then > I'm all for saying "good, keep it on a branch and don't merge it to > trunk". > > I was hoping we could all work together a bit better here... > contentious merge votes like this just cause cases where different > distros diverge by merging different branches way ahead of the > upstream (eg yours with windows support, ours with 347, etc) > > -Todd > -- > Todd Lipcon > Software Engineer, Cloudera On Wed, Feb 20, 2013 at 3:40 PM, Todd Lipcon wrote: > On Wed, Feb 20, 2013 at 3:31 PM, Suresh Srinivas wrote: >>> Given that this is an optimization, and we have a ton of optimizations >>> which don't yet run on Windows, I don't think that should be >>> considered. Additionally, the Windows support has not yet been merged, >>> nor is it in any release, so this isn't a regression. >>> >> >> This is a critical functionality for HBase peformance and an optimization >> we consider >> very important to have. > > Too bad it doesn't work in any of the normal installations... none of > the packages for Hadoop would allow it to work, given that the data > directories will be owned by HDFS and not world readable, and > tasks/HBase would run as an "hbase" user, which wouldn't have direct > access to the block files. > >>> >>> I would be happy to review an addition to the HDFS-347 branch which >>> addresses this issue. But I don't think we should be maintaining two >>> codepaths for the sake of an optimization on a platform which is not >>> yet officially supported on trunk, especially when the old code path >>> is _insecure_ and thus unusable in most environments. >> >> >> I have to disagree. No where in the jira or the design it is explicitly >> stated that >> the old short circuit functionality is being removed. My assumption has been >> that it will not be removed. > > I've tried this avenue in the past on other insecurities which were > fixed. Sorry if you were depending on insecure behavior. The project > should move on and not have 3+ ways of implementing the same thing. > >> As regards "officially supported", we have been doing >> windows development for >> more than a year. In fact branch-1-win is being used by a lot users. Given >> merging it to branch-1 requires first making it available in trunk, we have >> been doing >> a lot of work in branch-trunk-win. It is almost ready to be merged as well. >> >> I am -1 on removing existing short circuit until an alternative short >> circuit similar >> to HDFS-347 on all the platforms. > > Great -- are you committed to building this equivalent feature for > Windows, then? On what timeline? From my viewpoint, Windows isn't a > supported platform *right now*, so vetoing based on it seems > meritless. > > BTW, the posix_fadvise based readahead is an important optimization > for many workloads, too, but from what I can tell looking at the > Windows branch, it doesn't support it. There are other places in the > Windows branch where performance is going to be worse - eg disabling > the pipelined crc32c implementation will be a 2-3x hit on that code > path. > > No one has voted to merge Windows support, and if merging Windows > support means that, from now on, every _optimization_ must work on > Windows, I don't think I will be able to vote +1. The vast majority of > the community is _not_ running Windows, and I don't want to block > progress on the small number of developers who know how to program > against that platform. > > If that's the axe hanging over our head with the Windows branch, then > I'm all for saying "good, keep it on a branch and don't merge it to > trunk". > > I was hoping we could all work together a bit better here... > contentious merge votes like this just cause cases where different > distros diverge by merging different branches way ahead of the > upstream (eg yours with windows support, ours with 347, etc) > > -Todd > -- > Todd Lipcon > Software Engineer, Cloudera