[
https://issues.apache.org/jira/browse/HBASE-6778?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14285959#comment-14285959
]
stack commented on HBASE-6778:
------------------------------
bq. When you say that ChoreService should be at the top level I'm not quite
sure what you mean
At org.apache.hadoop.hbase package probably in hbase-common if there are no
dependencies on later modules (Would be good to not have dependencies on later
modules and get it into hbase-common if you can)
ChoreService should be at the top-level IMO. It is used by lots of subpackages.
You can't put it in a subpackage without another subpackage reaching over to
make use of it. Doing this ups our entanglement quotient.
bq. Would you be able to point me in the right direction here (or maybe there
is a commit that I could review to see how it's typically done).
When I wrote that comment, I had come across empty class comments: i.e. the
javadoc at the head of a class. As I reviewed, I saw you had good javadoc on
the methods. What is missing is a bit of metadoc on what this service does and
when to use it. That should go in class comment. Make sense?
bq. would adding the "@VisibleForTesting" annotation suffice, or is there more
to it than that?
Make the methods and classes package private. Tests in same package can still
access package private methods. This may not be possible though. In that
case, then you'd have to make the methods public, and then they need the
@VisibleForTesting annotation.
On 4. I do not follow what missing starttime implies. Can the chore not keep
this state -- whether it ran the start -- internal to itself? But sounds like
you need it exposed so you can figure when to up threads. Does ExecutorService
not expose methods you could make use of? Queued chores and how many current
threads? Could you do a heuristic based off these? Just a suggestion.
Maybe look at keep the reschedule internal rather than expose it since it adds
facility we will have to support and it is 'new'. Has anyone asked for it?
Great work [~jonathan.lawlor]
> Deprecate Chore; its a thread per task when we should have one thread to do
> all tasks
> -------------------------------------------------------------------------------------
>
> Key: HBASE-6778
> URL: https://issues.apache.org/jira/browse/HBASE-6778
> Project: HBase
> Issue Type: Bug
> Reporter: stack
> Assignee: Jonathan Lawlor
> Attachments: HBASE_6778_WIP_v1.patch, thread_dump_HMaster.local.out
>
>
> Should use something like ScheduledThreadPoolExecutor instead (Elliott said
> this first I think; J-D said something similar just now).
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
