Closed
Conversation
added 2 commits
June 1, 2018 15:12
…at we can have a priority queue of scans
Closed
keith-turner
added a commit
to keith-turner/accumulo
that referenced
this pull request
Jul 3, 2018
* Created plugin API for prioritizing scans that avoids accessing internal types. This should make user written comparators more stable over time. * Made it possible to configure a comparator for each scan executor. Was previously a single comparator type for all executors. * Made it possible to pass options for comparator creation * Made it possible to configure a per table dispatcher. This dispatcher chooses which scan executor to use. Dispatcher are user written plugins. * Fixed comparator type casting. I don't think the comparators in apache#510 worked because the Runnables were never the type expected. Also, abstracted all of this away from a user who may write a comparator.
keith-turner
added a commit
to keith-turner/accumulo
that referenced
this pull request
Jul 3, 2018
* Created plugin API for prioritizing scans that avoids accessing internal types. This should make user written comparators more stable over time. * Made it possible to configure a comparator for each scan executor. Was previously a single comparator type for all executors. * Made it possible to pass options for comparator creation * Made it possible to configure a per table dispatcher. This dispatcher chooses which scan executor to use. Dispatcher are user written plugins. * Fixed comparator type casting. I don't think the comparators in apache#510 worked because the Runnables were never the type expected. Also, abstracted all of this away from a user who may write a comparator.
keith-turner
reviewed
Jul 3, 2018
Contributor
keith-turner
left a comment
keith-turner
left a comment
There was a problem hiding this comment.
I created #549 as a replacement for this PR.
| public class DefaultSessionComparator extends SessionComparator { | ||
|
|
||
| @Override | ||
| public int compareSession(Session sessionA, Session sessionB) { |
Contributor
There was a problem hiding this comment.
I did not carry this forward in #549 for the following reasons :
- Its goal is not documented and its hard to understand the goal looking at the code.
- Its seems to prioritize newer scans with an exception for scans with "large" idle times. However, I think for two scans that have never run it should prioritize the oldest. But this goes back to the goal not being documented.
- I think there may be overflow bugs with casting of long to int, but maybe this was intentional?? Not sure.
| * we have to begin aging A | ||
| */ | ||
| if (idleTimeA > sessionA.maxIdleAccessTime) { | ||
| comparison = -1 * Long.valueOf(idleTimeA - maxIdle).intValue(); |
Contributor
There was a problem hiding this comment.
It seems that if we are here, then we know that
idleTimeA > sessionA.maxIdleAccessTimesessionA.maxIdleAccessTime >= maxIdle
Therefore it seems that idleTimeA - maxIdle will always be positive, unless there is some expectation with casting long to int??
Contributor
Author
|
closing as the changes are updated in #549 |
keith-turner
added a commit
that referenced
this pull request
Jul 10, 2018
* Created plugin API for prioritizing scans that avoids accessing internal types. This should make user written comparators more stable over time. * Made it possible to configure a comparator for each scan executor. Was previously a single comparator type for all executors. * Made it possible to pass options for comparator creation * Made it possible to configure a per table dispatcher. This dispatcher chooses which scan executor to use. Dispatcher are user written plugins. * Fixed comparator type casting. I don't think the comparators in #510 worked because the Runnables were never the type expected. Also, abstracted all of this away from a user who may write a comparator.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on #509. Acting as a proxy for @ marco polo to port changes from custom fork. Currently fails to pass verify due to conflict between format and checkstyle plugin. Format plugin will unwrap multi line declaration which makes checkstyle complain about line being too long.