[
https://issues.apache.org/jira/browse/HBASE-9754?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13798123#comment-13798123
]
Ted Yu edited comment on HBASE-9754 at 10/17/13 5:23 PM:
---------------------------------------------------------
bq. What are the implications of this change?
You were referring to the change in HBaseTestingUtility.java, right ?
Here is what resetThreadReadPoint() does:
{code}
- public static void resetThreadReadPoint() {
- perThreadReadPoint.set(0L);
- }
{code}
Meaning, effective readpoint is reset to 0. Hence passing 0 to
store.getScanner() is consistent with the existing test.
bq. Would think we'd retain at least the first line?
Is the first line MultiVersionConsistencyControl.resetThreadReadPoint() ? That
method has been removed.
bq. Does defaulting 0 in all tests change what is being tested
There is no change in semantics. 0 is only used when the two StoreScanner
ctors, marked for testing, are used.
bq. This should be final now?
Done.
bq. Is this an odd 'public' method to have in HRegion?
It is used by NoOpScanPolicyObserver. Compilation passes when I change it to
package private. My reasoning was that if user's RegionObserver overrides
preStoreScannerOpen(), this method is needed. User's code would not be in
org.apache.hadoop.hbase.regionserver package.
bq. It is the readPoint for the region?
Readpoint is associated with mvcc for the region.
bq. Undocumented passing of zero (explaining why this is ok)
Will add doc in next patch.
bq. and what smallest means in latter
Would do this too.
was (Author: yuzhih...@gmail.com):
bq. What are the implications of this change?
You were referring to the change in HBaseTestingUtility.java, right ?
Here is what resetThreadReadPoint() does:
- public static void resetThreadReadPoint() {
- perThreadReadPoint.set(0L);
- }
Meaning, effective readpoint is reset to 0. Hence passing 0 to
store.getScanner() is consistent with the existing test.
bq. Would think we'd retain at least the first line?
Is the first line MultiVersionConsistencyControl.resetThreadReadPoint() ? That
method has been removed.
bq. Does defaulting 0 in all tests change what is being tested
There is no change in semantics. 0 is only used when the two StoreScanner
ctors, marked for testing, are used.
bq. This should be final now?
Done.
bq. Is this an odd 'public' method to have in HRegion?
It is used by NoOpScanPolicyObserver. Compilation passes when I change it to
package private. My reasoning was that if user's RegionObserver overrides
preStoreScannerOpen(), this method is needed. User's code would not be in
org.apache.hadoop.hbase.regionserver package.
bq. It is the readPoint for the region?
Readpoint is associated with mvcc for the region.
bq. Undocumented passing of zero (explaining why this is ok)
Will add doc in next patch.
bq. and what smallest means in latter
Would do this too.
> Consider eliminating threadlocals from MVCC code
> ------------------------------------------------
>
> Key: HBASE-9754
> URL: https://issues.apache.org/jira/browse/HBASE-9754
> Project: HBase
> Issue Type: Bug
> Reporter: Lars Hofhansl
> Assignee: Ted Yu
> Fix For: 0.98.0
>
> Attachments: 9754-rp-0.txt, 9754-rp-1.txt, 9754-rp-hregion.txt,
> 9754-rp-hregion-v2.txt, 9754-rp-hregion-v3.txt
>
>
> Brought up by [~vrodionov] and [~yuzhih...@gmail.com].
> Currently we use ThreadLocals to communicate the current readpoint between a
> RegionScanner and the Store\{File}Scanner's down the stack.
> Since ThreadLocals are not cheap we should consider whether it is possible to
> pass the readpoint through the call stack instead.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
