hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mikael Sitruk <mikael.sit...@gmail.com>
Subject Re: question regarding code
Date Fri, 30 Dec 2011 00:15:34 GMT
Lars hi

This method will be called anytime that old log exist and might be cleaned
(HLog.cleanOldsLogs()) and there are too much logs.
It is called from LogRoller thread, Hlog creation, and metaUtils.shutdown()
(code in 0.90.1 - also present at least in those classes in trunk),
The creation of Arraylist is approx 24 bytes (8 for Arraylist object, 12
for the array member and 4 for the int member) if I didn't forgot
something, nevertheless the "if" will not be checked for each region having
an older log file (which will occurs a lot if you have heavy writes
scenarios).
This is true that this will create garbage (in case it will not be used)
but this is a short lived object, it will be collected immediately with a
minor garbage collection. It would have be more problematic if the object
was long lived.

Anyway you are the one to decide, if you accept a jira, i will open one,
just let me know.

Regards,
Mikael.S


On Thu, Dec 29, 2011 at 6:54 PM, lars hofhansl <lhofhansl@yahoo.com> wrote:

> The only downside is that the ArrayList now is always created, even if it
> is not needed, thereby producing unnecessary garbage.
> I have not bearing how frequently we'll get here and find no relevant
> regions, though.
>
>
> -- Lars
>
>
> ----- Original Message -----
> From: Harsh J <harsh@cloudera.com>
> To: dev@hbase.apache.org
> Cc:
> Sent: Thursday, December 29, 2011 12:51 AM
> Subject: Re: question regarding code
>
> Yeah that'd work too. File a JIRA with the change?
>
> On 29-Dec-2011, at 2:12 PM, Mikael Sitruk wrote:
>
> > Hi all
> >
> > I have question on some code (taken from HLog) see below
> >
> >
> >  static byte [][] findMemstoresWithEditsEqualOrOlderThan(final long
> > oldestWALseqid,
> >      final Map<byte [], Long> regionsToSeqids) {
> >    //  This method is static so it can be unit tested the easier.
> >    List<byte []> regions = null;
> >    for (Map.Entry<byte [], Long> e: regionsToSeqids.entrySet()) {
> >      if (e.getValue().longValue() <= oldestWALseqid) {
> >        if (regions == null) regions = new ArrayList<byte []>();
> >        regions.add(e.getKey());
> >      }
> >    }
> >    return regions == null?
> >      null: regions.toArray(new byte [][] {HConstants.EMPTY_BYTE_ARRAY});
> >  }
> >
> > Shouldn't be better to remove the if in the loop doing as follow?
> >
> >  static byte [][] findMemstoresWithEditsEqualOrOlderThan(final long
> > oldestWALseqid,
> >      final Map<byte [], Long> regionsToSeqids) {
> >    //  This method is static so it can be unit tested the easier.
> >    List<byte []> regions = new ArrayList<byte []>();
> >    for (Map.Entry<byte [], Long> e: regionsToSeqids.entrySet()) {
> >      if (e.getValue().longValue() <= oldestWALseqid) {
> >        //if (regions == null) regions = new ArrayList<byte []>();
> >        regions.add(e.getKey());
> >      }
> >    }
> >    return regions.size() == 0?
> >      null: regions.toArray(new byte [][] {HConstants.EMPTY_BYTE_ARRAY});
> >  }
> >
> > regards,
> >
> > Mikael.S
>



-- 
Mikael.S

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message