[jira] [Commented] (HBASE-14549) Simplify scanner stack reset logic
[ https://issues.apache.org/jira/browse/HBASE-14549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15383114#comment-15383114 ] stack commented on HBASE-14549: --- So, this stuff is no longer complex? Nothing to do based off your findings [~lhofhansl]? > Simplify scanner stack reset logic > -- > > Key: HBASE-14549 > URL: https://issues.apache.org/jira/browse/HBASE-14549 > Project: HBase > Issue Type: Bug >Reporter: Lars Hofhansl >Assignee: Lars Hofhansl > Attachments: 14549-0.98.txt > > > Looking at the code, I find that the logic is unnecessarily complex. > We indicate in updateReaders that the scanner stack needs to be reset. Then > almost all store scanner (and derived classes) methods need to check and > actually reset the scanner stack. > Compaction are rare, we should reset the scanner stack in update readers, and > hence avoid needing to check in all methods. > Patch forthcoming. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-14549) Simplify scanner stack reset logic
[ https://issues.apache.org/jira/browse/HBASE-14549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14951645#comment-14951645 ] Lars Hofhansl commented on HBASE-14549: --- OK... I get it now. In KeyValueHeap we always have the top scanner removed from the heap and store in this.current. In the case of a scanner stack reset, which used to be the top scanner may no longer be and we would have to put it back on the heap and pull a new top scanner out, but we have no way of knowing that. I.e. the top StoreScanner in the RegionScanner's KeyValueHeap might have changed without notice. This all looks a bit brittle. What if _other_ StoreScanners (other than the top scanner) change their peek-element? We'd have to remove and re-add to the RegionScanner's heap as well... I think. Seems the correct way would be to reset the RegionScanner stack whenever any of the stores have been compacted. That would also naturally coarsen the lock to RegionScanner, but the RegionScanner may not have all the information to reset all StoreScanners. Tricky... > Simplify scanner stack reset logic > -- > > Key: HBASE-14549 > URL: https://issues.apache.org/jira/browse/HBASE-14549 > Project: HBase > Issue Type: Bug >Reporter: Lars Hofhansl >Assignee: Lars Hofhansl > Attachments: 14549-0.98.txt > > > Looking at the code, I find that the logic is unnecessarily complex. > We indicate in updateReaders that the scanner stack needs to be reset. Then > almost all store scanner (and derived classes) methods need to check and > actually reset the scanner stack. > Compaction are rare, we should reset the scanner stack in update readers, and > hence avoid needing to check in all methods. > Patch forthcoming. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-14549) Simplify scanner stack reset logic
[ https://issues.apache.org/jira/browse/HBASE-14549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14946279#comment-14946279 ] Lars Hofhansl commented on HBASE-14549: --- Looking more at HBASE-5121, I think I do not understand what the issue is. We're recreating the scanner heap, how can there be state left over from the prior scan. I think as soon as I understand that, I can fix this one and make it simpler. > Simplify scanner stack reset logic > -- > > Key: HBASE-14549 > URL: https://issues.apache.org/jira/browse/HBASE-14549 > Project: HBase > Issue Type: Bug >Reporter: Lars Hofhansl >Assignee: Lars Hofhansl > Attachments: 14549-0.98.txt > > > Looking at the code, I find that the logic is unnecessarily complex. > We indicate in updateReaders that the scanner stack needs to be reset. Then > almost all store scanner (and derived classes) methods need to check and > actually reset the scanner stack. > Compaction are rare, we should reset the scanner stack in update readers, and > hence avoid needing to check in all methods. > Patch forthcoming. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-14549) Simplify scanner stack reset logic
[ https://issues.apache.org/jira/browse/HBASE-14549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14942876#comment-14942876 ] Lars Hofhansl commented on HBASE-14549: --- Thanks for keeping me honest [~stack] :) > Simplify scanner stack reset logic > -- > > Key: HBASE-14549 > URL: https://issues.apache.org/jira/browse/HBASE-14549 > Project: HBase > Issue Type: Bug >Reporter: Lars Hofhansl >Assignee: Lars Hofhansl > Attachments: 14549-0.98.txt > > > Looking at the code, I find that the logic is unnecessarily complex. > We indicate in updateReaders that the scanner stack needs to be reset. Then > almost all store scanner (and derived classes) methods need to check and > actually reset the scanner stack. > Compaction are rare, we should reset the scanner stack in update readers, and > hence avoid needing to check in all methods. > Patch forthcoming. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-14549) Simplify scanner stack reset logic
[ https://issues.apache.org/jira/browse/HBASE-14549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14942867#comment-14942867 ] Lars Hofhansl commented on HBASE-14549: --- See HBASE-5121 (and my _own_ suggestion to simplify fix there :) ). > Simplify scanner stack reset logic > -- > > Key: HBASE-14549 > URL: https://issues.apache.org/jira/browse/HBASE-14549 > Project: HBase > Issue Type: Bug >Reporter: Lars Hofhansl >Assignee: Lars Hofhansl > Attachments: 14549-0.98.txt > > > Looking at the code, I find that the logic is unnecessarily complex. > We indicate in updateReaders that the scanner stack needs to be reset. Then > almost all store scanner (and derived classes) methods need to check and > actually reset the scanner stack. > Compaction are rare, we should reset the scanner stack in update readers, and > hence avoid needing to check in all methods. > Patch forthcoming. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-14549) Simplify scanner stack reset logic
[ https://issues.apache.org/jira/browse/HBASE-14549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14942865#comment-14942865 ] Lars Hofhansl commented on HBASE-14549: --- Hmm... Yes, not quite right, although I do not understand why, yet. > Simplify scanner stack reset logic > -- > > Key: HBASE-14549 > URL: https://issues.apache.org/jira/browse/HBASE-14549 > Project: HBase > Issue Type: Bug >Reporter: Lars Hofhansl >Assignee: Lars Hofhansl > Attachments: 14549-0.98.txt > > > Looking at the code, I find that the logic is unnecessarily complex. > We indicate in updateReaders that the scanner stack needs to be reset. Then > almost all store scanner (and derived classes) methods need to check and > actually reset the scanner stack. > Compaction are rare, we should reset the scanner stack in update readers, and > hence avoid needing to check in all methods. > Patch forthcoming. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-14549) Simplify scanner stack reset logic
[ https://issues.apache.org/jira/browse/HBASE-14549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14942862#comment-14942862 ] Lars Hofhansl commented on HBASE-14549: --- TestScanner looks very relevant. Looking., > Simplify scanner stack reset logic > -- > > Key: HBASE-14549 > URL: https://issues.apache.org/jira/browse/HBASE-14549 > Project: HBase > Issue Type: Bug >Reporter: Lars Hofhansl >Assignee: Lars Hofhansl > Attachments: 14549-0.98.txt > > > Looking at the code, I find that the logic is unnecessarily complex. > We indicate in updateReaders that the scanner stack needs to be reset. Then > almost all store scanner (and derived classes) methods need to check and > actually reset the scanner stack. > Compaction are rare, we should reset the scanner stack in update readers, and > hence avoid needing to check in all methods. > Patch forthcoming. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-14549) Simplify scanner stack reset logic
[
https://issues.apache.org/jira/browse/HBASE-14549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14942848#comment-14942848
]
Hadoop QA commented on HBASE-14549:
---
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12764925/14549-0.98.txt
against 0.98 branch at commit 19045a5ea759392b5a6b8bd8001a3dd9dd5245fe.
ATTACHMENT ID: 12764925
{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.0 2.4.1 2.5.0 2.5.1 2.5.2 2.6.0 2.6.1 2.7.0
2.7.1)
{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
28 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 post-site goal succeeds with this patch.
{color:red}-1 core tests{color}. The patch failed these unit tests:
org.apache.hadoop.hbase.regionserver.TestScanner
Test results:
https://builds.apache.org/job/PreCommit-HBASE-Build/15871//testReport/
Release Findbugs (version 2.0.3)warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/15871//artifact/patchprocess/newFindbugsWarnings.html
Checkstyle Errors:
https://builds.apache.org/job/PreCommit-HBASE-Build/15871//artifact/patchprocess/checkstyle-aggregate.html
Javadoc warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/15871//artifact/patchprocess/patchJavadocWarnings.txt
Console output:
https://builds.apache.org/job/PreCommit-HBASE-Build/15871//console
This message is automatically generated.
> Simplify scanner stack reset logic
> --
>
> Key: HBASE-14549
> URL: https://issues.apache.org/jira/browse/HBASE-14549
> Project: HBase
> Issue Type: Bug
>Reporter: Lars Hofhansl
>Assignee: Lars Hofhansl
> Attachments: 14549-0.98.txt
>
>
> Looking at the code, I find that the logic is unnecessarily complex.
> We indicate in updateReaders that the scanner stack needs to be reset. Then
> almost all store scanner (and derived classes) methods need to check and
> actually reset the scanner stack.
> Compaction are rare, we should reset the scanner stack in update readers, and
> hence avoid needing to check in all methods.
> Patch forthcoming.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-14549) Simplify scanner stack reset logic
[ https://issues.apache.org/jira/browse/HBASE-14549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14942829#comment-14942829 ] stack commented on HBASE-14549: --- I looked a bit more... patch makes more sense now. +1 again if hadoopqa likes it. > Simplify scanner stack reset logic > -- > > Key: HBASE-14549 > URL: https://issues.apache.org/jira/browse/HBASE-14549 > Project: HBase > Issue Type: Bug >Reporter: Lars Hofhansl >Assignee: Lars Hofhansl > Attachments: 14549-0.98.txt > > > Looking at the code, I find that the logic is unnecessarily complex. > We indicate in updateReaders that the scanner stack needs to be reset. Then > almost all store scanner (and derived classes) methods need to check and > actually reset the scanner stack. > Compaction are rare, we should reset the scanner stack in update readers, and > hence avoid needing to check in all methods. > Patch forthcoming. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-14549) Simplify scanner stack reset logic
[ https://issues.apache.org/jira/browse/HBASE-14549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14942827#comment-14942827 ] stack commented on HBASE-14549: --- Not sure I understand but +1 on the change given this basis (smile). I'd think if you've broken something, it'll show in a test run. Let me submit. > Simplify scanner stack reset logic > -- > > Key: HBASE-14549 > URL: https://issues.apache.org/jira/browse/HBASE-14549 > Project: HBase > Issue Type: Bug >Reporter: Lars Hofhansl >Assignee: Lars Hofhansl > Attachments: 14549-0.98.txt > > > Looking at the code, I find that the logic is unnecessarily complex. > We indicate in updateReaders that the scanner stack needs to be reset. Then > almost all store scanner (and derived classes) methods need to check and > actually reset the scanner stack. > Compaction are rare, we should reset the scanner stack in update readers, and > hence avoid needing to check in all methods. > Patch forthcoming. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-14549) Simplify scanner stack reset logic
[ https://issues.apache.org/jira/browse/HBASE-14549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14942825#comment-14942825 ] Lars Hofhansl commented on HBASE-14549: --- There is a slight performance improvement too. > Simplify scanner stack reset logic > -- > > Key: HBASE-14549 > URL: https://issues.apache.org/jira/browse/HBASE-14549 > Project: HBase > Issue Type: Bug >Reporter: Lars Hofhansl >Assignee: Lars Hofhansl > Attachments: 14549-0.98.txt > > > Looking at the code, I find that the logic is unnecessarily complex. > We indicate in updateReaders that the scanner stack needs to be reset. Then > almost all store scanner (and derived classes) methods need to check and > actually reset the scanner stack. > Compaction are rare, we should reset the scanner stack in update readers, and > hence avoid needing to check in all methods. > Patch forthcoming. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-14549) Simplify scanner stack reset logic
[ https://issues.apache.org/jira/browse/HBASE-14549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14942557#comment-14942557 ] Lars Hofhansl commented on HBASE-14549: --- TestAtomicOperation passes multiple runs (it mixes flushes and compaction with active scanning) > Simplify scanner stack reset logic > -- > > Key: HBASE-14549 > URL: https://issues.apache.org/jira/browse/HBASE-14549 > Project: HBase > Issue Type: Bug >Reporter: Lars Hofhansl >Assignee: Lars Hofhansl > Attachments: 14549-0.98.txt > > > Looking at the code, I find that the logic is unnecessarily complex. > We indicate in updateReaders that the scanner stack needs to be reset. Than > almost all store scanner (and derived classes) methods need to check and > actually reset the scanner stack. > Compaction are rare, we should reset the scanner stack in update readers, and > hence avoid needing to check in all methods. > Patch forthcoming. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
