[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14391488#comment-14391488
]
Hadoop QA commented on HBASE-13378:
---
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12708755/HBASE-13378.txt
against master branch at commit 485800830a58da1af31beaac859d99b56d823f99.
ATTACHMENT ID: 12708755
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:red}-1 tests included{color}. The patch doesn't appear to include
any new or modified tests.
Please justify why no new tests are needed for this
patch.
Also please list what manual steps were performed to
verify this patch.
{color:green}+1 hadoop versions{color}. The patch compiles with all
supported hadoop versions (2.4.1 2.5.2 2.6.0)
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 protoc{color}. The applied patch does not increase the
total number of protoc compiler warnings.
{color:red}-1 javadoc{color}. The javadoc tool appears to have generated 4
warning messages.
{color:green}+1 checkstyle{color}. The applied patch does not increase the
total number of checkstyle errors
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 2.0.3) warnings.
{color:red}-1 release audit{color}. The applied patch generated 1 release
audit warnings (more than the master's current 0 warnings).
{color:green}+1 lineLengths{color}. The patch does not introduce lines
longer than 100
{color:green}+1 site{color}. The mvn site goal succeeds with this patch.
{color:red}-1 core tests{color}. The patch failed these unit tests:
{color:red}-1 core zombie tests{color}. There are 1 zombie test(s):
at
org.apache.hadoop.hbase.coprocessor.TestRegionServerObserver.testCoprocessorHooksInRegionsMerge(TestRegionServerObserver.java:100)
Test results:
https://builds.apache.org/job/PreCommit-HBASE-Build/13524//testReport/
Release audit warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/13524//artifact/patchprocess/patchReleaseAuditWarnings.txt
Release Findbugs (version 2.0.3)warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/13524//artifact/patchprocess/newFindbugsWarnings.html
Checkstyle Errors:
https://builds.apache.org/job/PreCommit-HBASE-Build/13524//artifact/patchprocess/checkstyle-aggregate.html
Javadoc warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/13524//artifact/patchprocess/patchJavadocWarnings.txt
Console output:
https://builds.apache.org/job/PreCommit-HBASE-Build/13524//console
This message is automatically generated.
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Attachments: HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14391513#comment-14391513
]
stack commented on HBASE-13378:
---
Patch LGTM. [~lhofhansl] this your area... you good with it?
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Attachments: HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14391532#comment-14391532
]
Lars Hofhansl commented on HBASE-13378:
---
Looks fine to me.
One interesting bit is: Now we won't hold on versions of Cells read by an
READ_UNCOMMITTED scanner (i.e. even while the scanner is active those cells can
be removed by a flush or a compaction).
Up to this point READ_UNCOMMITED meant: You might see partially finished rows.
Now it means: You might see partially finished rows, and you may not see cells
that have existed when the scanner started.
Not sure how important the 2nd part is, but up to now we guarantee that _all_
scanners see _all_ data existed when the scanner started. That would no longer
be the case.
I do think for READ_UNCOMMITTED that's OK... I.e. all bets are off in that
case, but it is a change in behavior.
[~eclark], Kannan and Karthik from Facebook add this back in the days, do you
have an opinion?
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Attachments: HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14547060#comment-14547060
]
Lars Hofhansl commented on HBASE-13378:
---
Hmm... Read mine as a -1 as I'm not sure the change in semantic is correct or
expected.
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Fix For: 2.0.0, 0.98.14, 1.0.2, 1.2.0, 1.1.1
>
> Attachments: HBASE-13378.patch, HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14547062#comment-14547062
]
Andrew Purtell commented on HBASE-13378:
"looks fine to me" is the oddest -1 I've ever seen. :-)
Anyway, ok
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Fix For: 2.0.0, 0.98.14, 1.0.2, 1.2.0, 1.1.1
>
> Attachments: HBASE-13378.patch, HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14547090#comment-14547090
]
Hadoop QA commented on HBASE-13378:
---
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12733365/HBASE-13378.patch
against master branch at commit f49111e5f8a2db8f3065188f03c7ad6d4411bd10.
ATTACHMENT ID: 12733365
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:red}-1 tests included{color}. The patch doesn't appear to include
any new or modified tests.
Please justify why no new tests are needed for this
patch.
Also please list what manual steps were performed to
verify this patch.
{color:green}+1 hadoop versions{color}. The patch compiles with all
supported hadoop versions (2.4.1 2.5.2 2.6.0)
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 protoc{color}. The applied patch does not increase the
total number of protoc compiler warnings.
{color:green}+1 javadoc{color}. The javadoc tool did not generate any
warning messages.
{color:green}+1 checkstyle{color}. The applied patch does not increase the
total number of checkstyle errors
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 2.0.3) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:green}+1 lineLengths{color}. The patch does not introduce lines
longer than 100
{color:green}+1 site{color}. The mvn site goal succeeds with this patch.
{color:green}+1 core tests{color}. The patch passed unit tests in .
Test results:
https://builds.apache.org/job/PreCommit-HBASE-Build/14069//testReport/
Release Findbugs (version 2.0.3)warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/14069//artifact/patchprocess/newFindbugsWarnings.html
Checkstyle Errors:
https://builds.apache.org/job/PreCommit-HBASE-Build/14069//artifact/patchprocess/checkstyle-aggregate.html
Console output:
https://builds.apache.org/job/PreCommit-HBASE-Build/14069//console
This message is automatically generated.
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Fix For: 2.0.0, 0.98.14, 1.0.2, 1.2.0, 1.1.1
>
> Attachments: HBASE-13378.patch, HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14547291#comment-14547291
]
Lars Hofhansl commented on HBASE-13378:
---
bq. "looks fine to me" is the oddest -1 I've ever seen.
Touché :)
It's only a -1 until we all agree that the change in behavior is acceptable.
Namely:
bq. Up to this point READ_UNCOMMITED meant: You might see partially finished
rows. Now it means: You might see partially finished rows, and you may not see
cells that have existed when the scanner started.
We never had the case before that a scanner could not see Cells that existed
when it started.
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Attachments: HBASE-13378.patch, HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14570992#comment-14570992
]
John Leach commented on HBASE-13378:
Seems like a nit IMO when you are comparing it to synchronizing all gets/scans
in HBase...
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Attachments: HBASE-13378.patch, HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14572024#comment-14572024
]
Andrew Purtell commented on HBASE-13378:
bq. Up to this point READ_UNCOMMITED meant: You might see partially finished
rows. Now it means: You might see partially finished rows, and you may not see
cells that have existed when the scanner started.
I'd be ok with this change in 0.98 with a release note describing the change in
semantics. In either case the user is asking for increased possible concurrency
in exchange for 'dirty' reads. What this patch does is alter a bit how dirty is
dirty, depending. (smile) I'm not sure what [~enis] or [~ndimiduk] would want
to do wrt. 1.0 and 1.1, or [~busbey] for 1.2.
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Attachments: HBASE-13378.patch, HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14572165#comment-14572165
]
Lars Hofhansl commented on HBASE-13378:
---
+0 on this change, but if it helps SpliceMachine and we're OK with the semantic
change, let's get it in.
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Attachments: HBASE-13378.patch, HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14573464#comment-14573464
]
Enis Soztutar commented on HBASE-13378:
---
bq. I'd be ok with this change in 0.98 with a release note describing the
change in semantics. In either case the user is asking for increased possible
concurrency in exchange for 'dirty' reads. What this patch does is alter a bit
how dirty is dirty, depending. (smile) I'm not sure what Enis Soztutar or Nick
Dimiduk would want to do wrt. 1.0 and 1.1, or Sean Busbey for 1.2.
I would put this in the "not a bug fix" bucket, and would not include in 1.0
and 1.1 release lines.
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Attachments: HBASE-13378.patch, HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14573477#comment-14573477
]
John Leach commented on HBASE-13378:
Lars do you have a codeline where the decision on the flush and compaction is
made? Maybe I can use a different mechanism there?
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Attachments: HBASE-13378.patch, HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14574733#comment-14574733
]
Sean Busbey commented on HBASE-13378:
-
I agree that it should not go into the 1.0 and 1.1 lines. I'd be fine with a
release noted change in behavior on 1.2. (though perhaps we should ask on user@
how surprising this will be?)
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Attachments: HBASE-13378.patch, HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14574735#comment-14574735
]
John Leach commented on HBASE-13378:
I will take a closer look this weekend and see if I can get it to not change
the guarantee while removing the synchronization.
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Attachments: HBASE-13378.patch, HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14577851#comment-14577851
]
Nick Dimiduk commented on HBASE-13378:
--
{quote}
Up to this point READ_UNCOMMITED meant: You might see partially finished rows.
Now it means: You might see partially finished rows, and you may not see cells
that have existed when the scanner started.
{quote}
This sounds to me like an incompatible change for a patch release. No good for
1.0 or 1.1, I think.
Ah, but [~enis] and [~busbey] have already said as much.
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Attachments: HBASE-13378.patch, HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14577884#comment-14577884
]
John Leach commented on HBASE-13378:
Can someone explain this one to me?
"One interesting bit is: Now we won't hold on versions of Cells read by an
READ_UNCOMMITTED scanner (i.e. even while the scanner is active those cells can
be removed by a flush or a compaction)."
The only bit I changed is whether you are in the scannerReadPts map and unless
my IDE is acting goofy, I do not see usages outside of RegionScannerImpl.
Lars?
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Attachments: HBASE-13378.patch, HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14579309#comment-14579309
]
Andrew Purtell commented on HBASE-13378:
{quote}
bq. Up to this point READ_UNCOMMITED meant: You might see partially finished
rows.
Now it means: You might see partially finished rows, and you may not see cells
that have existed when the scanner started.
This sounds to me like an incompatible change for a patch release
{quote}
Precisely, why does this sound like an *incompatible* change?
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Attachments: HBASE-13378.patch, HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14581259#comment-14581259
]
Lars Hofhansl commented on HBASE-13378:
---
bq. Can someone explain this one to me?
The effect is on the READ_UNCOMMITTED scanner, this patch won't record it's
readpoint in scannerReadPoints, and flushes and compactions might let go of
Cells that are newer than the scanner's readpoint.
I actually think that is _OK_, I just wanted to consensus, it seems [~enis] and
[~ndimiduk] are not OK with it.
[~enis], [~ndimiduk], the behavior I described above would _only_ effect the
scanner declared with READ_UNCOMMITTED. Do you still think this is a
compatibility issue?
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Attachments: HBASE-13378.patch, HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14581312#comment-14581312
]
Nick Dimiduk commented on HBASE-13378:
--
>From server.org
bq. 3. PATCH version when you make backwards-compatible bug fixes
and
bq. 6. Patch version Z (x.y.Z | x > 0) MUST be incremented if only backwards
compatible bug fixes are introduced. A bug fix is defined as an internal change
that fixes incorrect behavior.
Was the pervious behavior incorrect? I think this is a performance enhancement
that changes behavior in a backward-incompatible way. Someone depending on
READ_UNCOMMITTED scanners to record the readpoint will need to deploy different
versions of their code to run on, say 1.1.0 and 1.1.1. I think that means it's
not compatible for a patch release. For instance, if Phoenix was depending on
this behavior, we would reject this change as a patch release, but I think it
would be accepted for for a minor release.
READ_UNCOMMITTED is probably not a widely used feature, but it is a publicly
available feature. I think the semantics of our operations are just as precious
as our APIs. To be on the safe side, I'd prefer this go into branch-1 but not
branch-1.0 or branch-1.1.
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Attachments: HBASE-13378.patch, HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14582545#comment-14582545
]
Lars Hofhansl commented on HBASE-13378:
---
Fair enough. Also... Now that I think about it again. Maybe this isn't so
benign after all.
Is that even what you want [~jleach]? It means that as you use this scanner
HBase can at any point flush or compact the data away that the scanner should
see based in MVCC.
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Attachments: HBASE-13378.patch, HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14582547#comment-14582547
]
John Leach commented on HBASE-13378:
Can you point me to the line of code that uses the scannerReadPoints for
determining whether to flush or compact the data?
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Attachments: HBASE-13378.patch, HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-13378) RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
[
https://issues.apache.org/jira/browse/HBASE-13378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14582744#comment-14582744
]
Lars Hofhansl commented on HBASE-13378:
---
Follow the Money... Or {{HRegion.getSmallestReadPoint()}} :)
> RegionScannerImpl synchronized for READ_UNCOMMITTED Isolation Levels
>
>
> Key: HBASE-13378
> URL: https://issues.apache.org/jira/browse/HBASE-13378
> Project: HBase
> Issue Type: New Feature
>Reporter: John Leach
>Assignee: John Leach
>Priority: Minor
> Attachments: HBASE-13378.patch, HBASE-13378.txt
>
> Original Estimate: 2h
> Time Spent: 2h
> Remaining Estimate: 0h
>
> This block of code below coupled with the close method could be changed so
> that READ_UNCOMMITTED does not synchronize.
> {CODE:JAVA}
> // synchronize on scannerReadPoints so that nobody calculates
> // getSmallestReadPoint, before scannerReadPoints is updated.
> IsolationLevel isolationLevel = scan.getIsolationLevel();
> synchronized(scannerReadPoints) {
> this.readPt = getReadpoint(isolationLevel);
> scannerReadPoints.put(this, this.readPt);
> }
> {CODE}
> This hotspots for me under heavy get requests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
