[
https://issues.apache.org/jira/browse/HBASE-12012?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14254305#comment-14254305
]
stack commented on HBASE-12012:
-------------------------------
bq. As I said, it's my opinion. You have a different opinion. That's all.
I wasn't expressing opinion. I was just asking a question why -- thought you'd
have a reason -- and trying to help by making a suggestion. Lets avoid
'opinion'.
bq. I preferred the single 'l' for obvious reasons.
I don't get why it is obvious.
I was just looking at your patch, not wikipedia on the english language.
Usually folks aim to be consistent. In the patch, there is already a method
that does is cancelled and it is spelled ResultBoundedCompletionService
#isCancelled. I thought you'd want to align. The old java Cancellable Interface
that I quoted you also spells it so (git blame says it was named neither by an
Englishman nor an American, but by a Frenchman)
(Not of your making -- you are just moving a class -- but the patch has this in
it:
{code}
90 public boolean isCancelled() {
91 return canceled;
{code}
... which is not right).
bq, Sure. The startCancel was already there and I kept the same name (grep for
startCancel in the patch and you will know what I mean)
That makes sense...especially as it seems to have bubbled up from pb
RpcController.
bq. This patch has been tested manually. It mainly brings the Scan cancellation
at par with how Get cancellation works - not much net new functionality.
I am suggesting you add a test so we do not regress on this new facility. The
rpc chassis is about to be swapped out. See HBASE-12684. It would be a shame
if we dropped your work on the floor by mistake.
bq. If you absolutely want me to change this to a Decoration, let me know.
It is not about what I want. Was just making a suggestion. Just trying to help
out.
This:
class ReplicaRegionServerCallable extends RegionServerCallable<Result>
implements Cancellable {
seems cleaner than this:
class ReplicaRegionServerCallable extends RegionServerCallable<Result>
implements CancellableCallable<Result> {
Hanging a Stoppable or Abortable Decoration on an operation is how we do it
elsewhere in the codebase. Cancellable just seemed to fit that mold.
Should RegionServerCallable ever NOT be cancellable?
To land the patch, I'd suggest you change isCancelable in your Interface -- or
change the Frenchmans' work to have one el only -- and explain how you did the
manual testing so that another coming after can try the same to make sure they
have not broken this functionality (though a test would be better -- are there
tests for cancelling callables elsewhere? If so, would they cover?).
> Improve cancellation for the scan RPCs
> --------------------------------------
>
> Key: HBASE-12012
> URL: https://issues.apache.org/jira/browse/HBASE-12012
> Project: HBase
> Issue Type: Sub-task
> Reporter: Devaraj Das
> Assignee: Devaraj Das
> Fix For: 2.0.0, 1.1.0
>
> Attachments: 12012-1.txt, 12012-2.txt, 12012-3.txt, 12012-4.txt
>
>
> Similar to HBASE-11564 but for scans.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
