You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@hbase.apache.org by lars hofhansl <lh...@yahoo.com> on 2011/08/06 08:13:22 UTC

Quick question about HRegionServer.addScanner(...)

I see this code in there:


    scannerId = rand.nextLong();
    String scannerName = String.valueOf(scannerId);
    scanners.put(scannerName, s);


It would be rare that rand.nextLong() would return the same id while an older scanner is still in progress, but if that happens,
it seems the result would be pretty bad (i.e. the old client scanner now returning result from a different server scanner).
We could just have an incrementing long (with 2^63, we'll never run our of numbers, and if we're worried about that we could reset the counter every time the scanners map is empty).


I am probably not understanding this sufficiently, yet... Hence the question.


Also why is scanners declared as:
  final Map<String, InternalScanner> scanners =
    new ConcurrentHashMap<String, InternalScanner>();


And not as 

  final Map<Long, InternalScanner> scanners =
    new ConcurrentHashMap<Long, InternalScanner>();


All places except (ScannerListener) receive a long and calls either String.valueOf or Long.toString to convert the passed long to a String.
Using Long still does autoboxing but it still seems better. Is the motivation to more descriptive names in the future?


Thanks.

-- Lars

Re: Quick question about HRegionServer.addScanner(...)

Posted by lars hofhansl <lh...@yahoo.com>.
Created https://issues.apache.org/jira/browse/HBASE-4178

(Although there I kinda reasoned that because 2^64 - we'd also use negative numbers - is so large that it might just be a hypothetical problem).



________________________________
From: Stack <st...@duboce.net>
To: user@hbase.apache.org; lars hofhansl <lh...@yahoo.com>
Sent: Monday, August 8, 2011 4:07 PM
Subject: Re: Quick question about HRegionServer.addScanner(...)

On Fri, Aug 5, 2011 at 11:13 PM, lars hofhansl <lh...@yahoo.com> wrote:
> I see this code in there:
>
>
>     scannerId = rand.nextLong();
>     String scannerName = String.valueOf(scannerId);
>     scanners.put(scannerName, s);
>
>
> It would be rare that rand.nextLong() would return the same id while an older scanner is still in progress, but if that happens,
> it seems the result would be pretty bad (i.e. the old client scanner now returning result from a different server scanner).
> We could just have an incrementing long (with 2^63, we'll never run our of numbers, and if we're worried about that we could reset the counter every time the scanners map is empty).
>

This seems like a good idea.  Mind filing a JIRA Lars?


> I am probably not understanding this sufficiently, yet... Hence the question.
>

Your understanding looks fine to me.


>
> Also why is scanners declared as:
>   final Map<String, InternalScanner> scanners =
>     new ConcurrentHashMap<String, InternalScanner>();
>
>
> And not as
>
>   final Map<Long, InternalScanner> scanners =
>     new ConcurrentHashMap<Long, InternalScanner>();
>
>
> All places except (ScannerListener) receive a long and calls either String.valueOf or Long.toString to convert the passed long to a String.
> Using Long still does autoboxing but it still seems better. Is the motivation to more descriptive names in the future?
>

I do not know why it does the above.   It seems off to me (May be
vestige of an incomplete refactoring).

Thanks for digging in here Lars,
St.Ack

Re: Quick question about HRegionServer.addScanner(...)

Posted by Stack <st...@duboce.net>.
On Fri, Aug 5, 2011 at 11:13 PM, lars hofhansl <lh...@yahoo.com> wrote:
> I see this code in there:
>
>
>     scannerId = rand.nextLong();
>     String scannerName = String.valueOf(scannerId);
>     scanners.put(scannerName, s);
>
>
> It would be rare that rand.nextLong() would return the same id while an older scanner is still in progress, but if that happens,
> it seems the result would be pretty bad (i.e. the old client scanner now returning result from a different server scanner).
> We could just have an incrementing long (with 2^63, we'll never run our of numbers, and if we're worried about that we could reset the counter every time the scanners map is empty).
>

This seems like a good idea.  Mind filing a JIRA Lars?


> I am probably not understanding this sufficiently, yet... Hence the question.
>

Your understanding looks fine to me.


>
> Also why is scanners declared as:
>   final Map<String, InternalScanner> scanners =
>     new ConcurrentHashMap<String, InternalScanner>();
>
>
> And not as
>
>   final Map<Long, InternalScanner> scanners =
>     new ConcurrentHashMap<Long, InternalScanner>();
>
>
> All places except (ScannerListener) receive a long and calls either String.valueOf or Long.toString to convert the passed long to a String.
> Using Long still does autoboxing but it still seems better. Is the motivation to more descriptive names in the future?
>

I do not know why it does the above.   It seems off to me (May be
vestige of an incomplete refactoring).

Thanks for digging in here Lars,
St.Ack