My initial question was about MetaUtils.ScannerListener but I can see that it went over that ;)
Since no-one is against keeping off line merges for now, I will apply my modifications into it directly. I will not put ScannerListerner back to MetaUtils since Merge seems to be the only class to use it. JMS 2013/3/22 Nick Dimiduk <[email protected]>: > Err, "an *empty* source file". > > On Fri, Mar 22, 2013 at 4:33 PM, Nick Dimiduk <[email protected]> wrote: > >> On Fri, Mar 22, 2013 at 8:19 AM, Jonathan Hsieh <[email protected]> wrote: >> >>> Let me offer a counter argument. The offline splitting code is still >>> present even though online splitting isn't the problem it used to be. We >>> actually added an extenal wal replayer even though we have wal replay in >>> our normal recovery path. Copy table still exists even though snapshots >>> and snapshot export exists. Would we consider removing these? >>> >> >> Somewhat tangential to this thread, I think we shouldn't be shy about >> deleting old/stale/deprecated code. This should be done on a case-by-case >> basis, of course. For example, in this case, maybe the snapshot >> functionality will evolve such that the CopyTable interface and semantics >> become a pure subset of snapshots. At that point, we should absolutely >> delete CopyTable as an independent code-path. >> >> As they say, "an source file ships no bugs". >> >> Just my 2¢ >> -n >> >> On Thu, Mar 21, 2013 at 7:07 PM, Jean-Marc Spaggiari < >>> [email protected]> wrote: >>> >>> > Hi Enis, >>> > >>> > I totaly agree. But even if online merge are available, maybe offline >>> > merges can still usefull in case the cluster is down for maintenance, >>> > or because there is any issue to start it, or anything else? We have >>> > it, so we should maybe try to keep it? >>> > >>> > JM >>> > >>> > 2013/3/21 Enis Söztutar <[email protected]>: >>> > > Thanks J-M. >>> > > >>> > > What I am trying to understand is that whether we should cut the cord >>> for >>> > > offline merge once online is working. If you think about it, there >>> should >>> > > not be a need to merge offline tables. >>> > > >>> > > Enis >>> > > >>> > > >>> > > On Thu, Mar 21, 2013 at 3:16 PM, Jean-Marc Spaggiari < >>> > > [email protected]> wrote: >>> > > >>> > >> Offline merge is already there and working fine. >>> > >> >>> > >> The usecase here was to retreive all the regions for a given table to >>> > >> merge them 2 by 2, offline. >>> > >> >>> > >> It's working fine, but since the Meta rework it's not working anymore >>> > >> and I'm trying to rebase the patch. >>> > >> >>> > >> Like J-D is saying, yes, it's used only in the offline merge... And >>> > >> since the online merge is coming, I think it's cleaner to keep the >>> > >> code in the offline merge since it will disapear soon, but in the >>> > >> meantime, at least, we will have the offline one. >>> > >> >>> > >> JM >>> > >> >>> > >> 2013/3/21 Enis Söztutar <[email protected]>: >>> > >> > What is the use case behind offline merge? Is it because we cannot >>> do >>> > >> > online merge yet? If we can get HBASE-7403 in, is there still need >>> to >>> > >> > support offline merge? >>> > >> > >>> > >> > Enis >>> > >> > >>> > >> > >>> > >> > On Thu, Mar 21, 2013 at 2:56 PM, Jean-Daniel Cryans < >>> > [email protected] >>> > >> >wrote: >>> > >> > >>> > >> >> As far as I can tell, only the merge code uses MetaUtils to do >>> > offline >>> > >> >> work. If this is the code you are in then pull it back into >>> MetaUtils >>> > >> >> I think. >>> > >> >> >>> > >> >> J-D >>> > >> >> >>> > >> >> On Thu, Mar 21, 2013 at 2:44 PM, Jean-Marc Spaggiari >>> > >> >> <[email protected]> wrote: >>> > >> >> > Vector is because of a very old bad habit ;) I will change that >>> to >>> > >> >> ArrayList. >>> > >> >> > >>> > >> >> > So far I have inlined the scanMetaRegion feature into the Merge, >>> > but >>> > >> >> > maybe it should be cleaner to put it back in? >>> > >> >> > >>> > >> >> > Anyway, I will keep the inlined one until everything is cleaned. >>> > >> >> > >>> > >> >> > 2013/3/21 Enis Söztutar <[email protected]>: >>> > >> >> >> The problem around current META scanning is that there is more >>> > than >>> > >> one >>> > >> >> way >>> > >> >> >> to do these, and the META layout is exposed. We should refrain >>> > from >>> > >> >> >> exposing the META details. >>> > >> >> >> AFAIK, these do the same thing: >>> > >> >> >> MetaReader.Visitor >>> > >> >> >> MetaScanner.MetaScannerVisitor, and >>> > >> >> >> MetaUtils.ScannerListener >>> > >> >> >> >>> > >> >> >> More concerning is that the code for managing META is spread >>> over >>> > >> >> >> MetaEditor, MetaReader, MetaScanner, MetaUtils, HRegionInfo >>> (and >>> > >> maybe >>> > >> >> >> more). There are a couple of issues to rework these interfaces, >>> > but I >>> > >> >> did >>> > >> >> >> not get the chance to work on those. >>> > >> >> >> >>> > >> >> >> >>> > >> >> >> Enis >>> > >> >> >> >>> > >> >> >> >>> > >> >> >> >>> > >> >> >> >>> > >> >> >> >>> > >> >> >> >>> > >> >> >> >>> > >> >> >> >>> > >> >> >> On Thu, Mar 21, 2013 at 2:18 PM, Jean-Daniel Cryans < >>> > >> >> [email protected]>wrote: >>> > >> >> >> >>> > >> >> >>> Mmmm I may have been trigger happy. You could pull back the >>> > >> >> >>> ScannerListener and scanMetaRegion. >>> > >> >> >>> >>> > >> >> >>> BTW, why are you using a Vector instead of ArrayList? >>> > >> >> >>> >>> > >> >> >>> J-D >>> > >> >> >>> >>> > >> >> >>> On Thu, Mar 21, 2013 at 2:05 PM, Jean-Marc Spaggiari >>> > >> >> >>> <[email protected]> wrote: >>> > >> >> >>> > Hi, >>> > >> >> >>> > >>> > >> >> >>> > In trunk, since HBASE-3171 (Drop ROOT and instead store META >>> > >> >> >>> > location(s) directly in ZooKeeper ) there is no more >>> > >> >> >>> > MetaUtils.ScannerListener. >>> > >> >> >>> > >>> > >> >> >>> > In the merge, I used it to retreive all the regions >>> belonging >>> > to a >>> > >> >> >>> > specific table, from the META. kind of scan. >>> > >> >> >>> > >>> > >> >> >>> > // Retrieve the list of regions for this table. >>> > >> >> >>> > final List<HRegionInfo> regions = new >>> > >> >> Vector<HRegionInfo>(); >>> > >> >> >>> > >>> > utils.scanMetaRegion(HRegionInfo.FIRST_META_REGIONINFO, >>> > >> new >>> > >> >> >>> > MetaUtils.ScannerListener() { >>> > >> >> >>> > public boolean processRow(HRegionInfo info) { >>> > >> >> >>> > if ((info != null) && >>> > >> >> >>> > (Bytes.compareTo(info.getTableName(), tableName) == 0)) { >>> > >> >> >>> > regions.add(info); >>> > >> >> >>> > } >>> > >> >> >>> > return true; >>> > >> >> >>> > } >>> > >> >> >>> > }); >>> > >> >> >>> > >>> > >> >> >>> > >>> > >> >> >>> > Is there a recommanded way to replace that? The Merge is >>> > running >>> > >> >> >>> > offline, so I can't do a scan. >>> > >> >> >>> > >>> > >> >> >>> > Thanks, >>> > >> >> >>> > >>> > >> >> >>> > JM >>> > >> >> >>> >>> > >> >> >>> > >> >>> > >>> >>> >>> >>> -- >>> // Jonathan Hsieh (shay) >>> // Software Engineer, Cloudera >>> // [email protected] >>> >> >>
