You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by "William Slacum (JIRA)" <ji...@apache.org> on 2012/07/06 05:39:33 UTC
[jira] [Created] (ACCUMULO-675) WrappingIterator's seenSeek should
be protected
William Slacum created ACCUMULO-675:
---------------------------------------
Summary: WrappingIterator's seenSeek should be protected
Key: ACCUMULO-675
URL: https://issues.apache.org/jira/browse/ACCUMULO-675
Project: Accumulo
Issue Type: Bug
Components: tserver
Affects Versions: 1.5.0, 1.4.1, 1.5.0-SNAPSHOT, 1.4.0
Environment: OSX, Linux
Reporter: William Slacum
Assignee: William Slacum
Priority: Trivial
Fix For: 1.5.0-SNAPSHOT
In 1.3, the WrappingIterator was pretty much some boilerplate code. In 1.4 on, a package private boolean called seenSeek was added to help enforce the iterator contract.
This causes some issues with iterators written for 1.3 and before, because the seenSeek property can't be set by an iterator outside of the core.iterators package, which is locked down. This means that sub iterators must always delegate up to the WrappingIterator's seek() method, even if implementors want to completely override seek().
I would like to provide more documentation on the WrappingIterator and make the seenSeek property protected so implementors don't need conditional logic to make the call to super.seek().
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Resolved] (ACCUMULO-675) WrappingIterator's seenSeek should
be protected
Posted by "Billie Rinaldi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ACCUMULO-675?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Billie Rinaldi resolved ACCUMULO-675.
-------------------------------------
Resolution: Fixed
Fix Version/s: (was: 1.5.0-SNAPSHOT)
1.4.2
1.5.0
> WrappingIterator's seenSeek should be protected
> -----------------------------------------------
>
> Key: ACCUMULO-675
> URL: https://issues.apache.org/jira/browse/ACCUMULO-675
> Project: Accumulo
> Issue Type: Bug
> Components: tserver
> Affects Versions: 1.5.0, 1.4.1, 1.5.0-SNAPSHOT, 1.4.0
> Environment: OSX, Linux
> Reporter: William Slacum
> Assignee: William Slacum
> Priority: Trivial
> Fix For: 1.5.0, 1.4.2
>
> Attachments: wrapping-iterator-mod.patch
>
> Original Estimate: 1m
> Remaining Estimate: 1m
>
> In 1.3, the WrappingIterator was pretty much some boilerplate code. In 1.4 on, a package private boolean called seenSeek was added to help enforce the iterator contract.
> This causes some issues with iterators written for 1.3 and before, because the seenSeek property can't be set by an iterator outside of the core.iterators package, which is locked down. This means that sub iterators must always delegate up to the WrappingIterator's seek() method, even if implementors want to completely override seek().
> I would like to provide more documentation on the WrappingIterator and make the seenSeek property protected so implementors don't need conditional logic to make the call to super.seek().
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (ACCUMULO-675) WrappingIterator's seenSeek
should be protected
Posted by "William Slacum (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ACCUMULO-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13409745#comment-13409745 ]
William Slacum commented on ACCUMULO-675:
-----------------------------------------
Because the WrappingIterator just ends up calling source.seek(...) there really isn't a use case that excludes that. It was just a surprise for me to find that `getSource().seek(...)` and `super.seek(...)` are longer equivalent, so backwards compatibility with 1.3 was broken.
If WrappingIterator's seek will only ever delegate to the source's seek method, then you're right, there really isn't an issue. But, the checks in place can be just as easily circumvented by overriding any of the other methods. At the very least making seenSeek protected will allow clients to make an effort to properly adhere to the contract, rather than simply avoiding the checks by overriding methods.
> WrappingIterator's seenSeek should be protected
> -----------------------------------------------
>
> Key: ACCUMULO-675
> URL: https://issues.apache.org/jira/browse/ACCUMULO-675
> Project: Accumulo
> Issue Type: Bug
> Components: tserver
> Affects Versions: 1.5.0, 1.4.1, 1.5.0-SNAPSHOT, 1.4.0
> Environment: OSX, Linux
> Reporter: William Slacum
> Assignee: William Slacum
> Priority: Trivial
> Fix For: 1.5.0-SNAPSHOT
>
> Attachments: wrapping-iterator-mod.patch
>
> Original Estimate: 1m
> Remaining Estimate: 1m
>
> In 1.3, the WrappingIterator was pretty much some boilerplate code. In 1.4 on, a package private boolean called seenSeek was added to help enforce the iterator contract.
> This causes some issues with iterators written for 1.3 and before, because the seenSeek property can't be set by an iterator outside of the core.iterators package, which is locked down. This means that sub iterators must always delegate up to the WrappingIterator's seek() method, even if implementors want to completely override seek().
> I would like to provide more documentation on the WrappingIterator and make the seenSeek property protected so implementors don't need conditional logic to make the call to super.seek().
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (ACCUMULO-675) WrappingIterator's seenSeek
should be protected
Posted by "Billie Rinaldi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ACCUMULO-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13410467#comment-13410467 ]
Billie Rinaldi commented on ACCUMULO-675:
-----------------------------------------
I'm in favor of the javadocs explaining how the usage of WrappingIterator has changed. If subclasses are allowed to change seenSeek, I could see people getting an error when they do something like findTop() in the init method, and just setting seenSeek to true to get rid of the error without learning about the iterator lifecycle. Perhaps this is misguided, but I think at this stage we want to draw as much attention as possible to the fact that we're trying to define the iterator lifecycle more clearly. I could see backing this off at some point once people have become familiar with the lifecycle. We could even consider removing these checks from WrappingIterator eventually, although they've been useful for making sure our internal iterators are doing the right things.
What do you think about the following?
{noformat}
/**
* A convenience class for implementing iterators that select, but do not modify, entries read from a source iterator. Default implementations exist for all
* methods, but {@link #deepCopy} will throw an <code>UnsupportedOperationException</code>.
*
* This iterator has some checks in place to enforce the iterator contract. Specifically, it verifies that it has a source iterator and that {@link #seek} has
* been called before any data is read. If either of these conditions does not hold true, an <code>IllegalStateException</code> will be thrown. In particular,
* this means that <code>getSource().seek</code> and <code>super.seek</code> no longer perform identical actions. Implementors should take note of this and if
* <code>seek</code> is overridden, ensure that <code>super.seek</code> is called before data is read.
*/
{noformat}
> WrappingIterator's seenSeek should be protected
> -----------------------------------------------
>
> Key: ACCUMULO-675
> URL: https://issues.apache.org/jira/browse/ACCUMULO-675
> Project: Accumulo
> Issue Type: Bug
> Components: tserver
> Affects Versions: 1.5.0, 1.4.1, 1.5.0-SNAPSHOT, 1.4.0
> Environment: OSX, Linux
> Reporter: William Slacum
> Assignee: William Slacum
> Priority: Trivial
> Fix For: 1.5.0-SNAPSHOT
>
> Attachments: wrapping-iterator-mod.patch
>
> Original Estimate: 1m
> Remaining Estimate: 1m
>
> In 1.3, the WrappingIterator was pretty much some boilerplate code. In 1.4 on, a package private boolean called seenSeek was added to help enforce the iterator contract.
> This causes some issues with iterators written for 1.3 and before, because the seenSeek property can't be set by an iterator outside of the core.iterators package, which is locked down. This means that sub iterators must always delegate up to the WrappingIterator's seek() method, even if implementors want to completely override seek().
> I would like to provide more documentation on the WrappingIterator and make the seenSeek property protected so implementors don't need conditional logic to make the call to super.seek().
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (ACCUMULO-675) WrappingIterator's seenSeek should
be protected
Posted by "William Slacum (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ACCUMULO-675?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
William Slacum updated ACCUMULO-675:
------------------------------------
Attachment: wrapping-iterator-mod.patch
Added documentation to WrappingIterator and made seenSeek's visibility protected.
> WrappingIterator's seenSeek should be protected
> -----------------------------------------------
>
> Key: ACCUMULO-675
> URL: https://issues.apache.org/jira/browse/ACCUMULO-675
> Project: Accumulo
> Issue Type: Bug
> Components: tserver
> Affects Versions: 1.5.0, 1.4.1, 1.5.0-SNAPSHOT, 1.4.0
> Environment: OSX, Linux
> Reporter: William Slacum
> Assignee: William Slacum
> Priority: Trivial
> Fix For: 1.5.0-SNAPSHOT
>
> Attachments: wrapping-iterator-mod.patch
>
> Original Estimate: 1m
> Remaining Estimate: 1m
>
> In 1.3, the WrappingIterator was pretty much some boilerplate code. In 1.4 on, a package private boolean called seenSeek was added to help enforce the iterator contract.
> This causes some issues with iterators written for 1.3 and before, because the seenSeek property can't be set by an iterator outside of the core.iterators package, which is locked down. This means that sub iterators must always delegate up to the WrappingIterator's seek() method, even if implementors want to completely override seek().
> I would like to provide more documentation on the WrappingIterator and make the seenSeek property protected so implementors don't need conditional logic to make the call to super.seek().
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (ACCUMULO-675) WrappingIterator's seenSeek should
be protected
Posted by "Christopher Tubbs (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ACCUMULO-675?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Christopher Tubbs updated ACCUMULO-675:
---------------------------------------
Affects Version/s: (was: 1.5.0)
> WrappingIterator's seenSeek should be protected
> -----------------------------------------------
>
> Key: ACCUMULO-675
> URL: https://issues.apache.org/jira/browse/ACCUMULO-675
> Project: Accumulo
> Issue Type: Bug
> Components: tserver
> Affects Versions: 1.4.1, 1.4.0
> Environment: OSX, Linux
> Reporter: William Slacum
> Assignee: William Slacum
> Priority: Trivial
> Fix For: 1.5.0, 1.4.2
>
> Attachments: wrapping-iterator-mod.patch
>
> Original Estimate: 1m
> Remaining Estimate: 1m
>
> In 1.3, the WrappingIterator was pretty much some boilerplate code. In 1.4 on, a package private boolean called seenSeek was added to help enforce the iterator contract.
> This causes some issues with iterators written for 1.3 and before, because the seenSeek property can't be set by an iterator outside of the core.iterators package, which is locked down. This means that sub iterators must always delegate up to the WrappingIterator's seek() method, even if implementors want to completely override seek().
> I would like to provide more documentation on the WrappingIterator and make the seenSeek property protected so implementors don't need conditional logic to make the call to super.seek().
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (ACCUMULO-675) WrappingIterator's seenSeek
should be protected
Posted by "William Slacum (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ACCUMULO-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13420055#comment-13420055 ]
William Slacum commented on ACCUMULO-675:
-----------------------------------------
Looks good to me.
> WrappingIterator's seenSeek should be protected
> -----------------------------------------------
>
> Key: ACCUMULO-675
> URL: https://issues.apache.org/jira/browse/ACCUMULO-675
> Project: Accumulo
> Issue Type: Bug
> Components: tserver
> Affects Versions: 1.5.0, 1.4.1, 1.5.0-SNAPSHOT, 1.4.0
> Environment: OSX, Linux
> Reporter: William Slacum
> Assignee: William Slacum
> Priority: Trivial
> Fix For: 1.5.0-SNAPSHOT
>
> Attachments: wrapping-iterator-mod.patch
>
> Original Estimate: 1m
> Remaining Estimate: 1m
>
> In 1.3, the WrappingIterator was pretty much some boilerplate code. In 1.4 on, a package private boolean called seenSeek was added to help enforce the iterator contract.
> This causes some issues with iterators written for 1.3 and before, because the seenSeek property can't be set by an iterator outside of the core.iterators package, which is locked down. This means that sub iterators must always delegate up to the WrappingIterator's seek() method, even if implementors want to completely override seek().
> I would like to provide more documentation on the WrappingIterator and make the seenSeek property protected so implementors don't need conditional logic to make the call to super.seek().
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (ACCUMULO-675) WrappingIterator's seenSeek
should be protected
Posted by "Billie Rinaldi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ACCUMULO-675?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13409680#comment-13409680 ]
Billie Rinaldi commented on ACCUMULO-675:
-----------------------------------------
I'm not convinced it's necessary to make seenSeek protected; it seems like it will just make it easier for iterators to circumvent the checking that is done to ensure the proper iterator lifecycle is observed. Do you have a specific use case where you can't call super.seek instead?
> WrappingIterator's seenSeek should be protected
> -----------------------------------------------
>
> Key: ACCUMULO-675
> URL: https://issues.apache.org/jira/browse/ACCUMULO-675
> Project: Accumulo
> Issue Type: Bug
> Components: tserver
> Affects Versions: 1.5.0, 1.4.1, 1.5.0-SNAPSHOT, 1.4.0
> Environment: OSX, Linux
> Reporter: William Slacum
> Assignee: William Slacum
> Priority: Trivial
> Fix For: 1.5.0-SNAPSHOT
>
> Attachments: wrapping-iterator-mod.patch
>
> Original Estimate: 1m
> Remaining Estimate: 1m
>
> In 1.3, the WrappingIterator was pretty much some boilerplate code. In 1.4 on, a package private boolean called seenSeek was added to help enforce the iterator contract.
> This causes some issues with iterators written for 1.3 and before, because the seenSeek property can't be set by an iterator outside of the core.iterators package, which is locked down. This means that sub iterators must always delegate up to the WrappingIterator's seek() method, even if implementors want to completely override seek().
> I would like to provide more documentation on the WrappingIterator and make the seenSeek property protected so implementors don't need conditional logic to make the call to super.seek().
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira