[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)