[
https://issues.apache.org/jira/browse/HBASE-6778?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14284465#comment-14284465
]
stack commented on HBASE-6778:
------------------------------
Nice writeup [~jonathan.lawlor]
ChoreService should be at the top-level I believe as is Chore (right?) It is
used all over the code base.
Before final patch, backfill doc. Above outline would be good skeleton. For
example, nothing currently on ChoreService but that is because this a WIP.
Doc on data members is good quality.
Not needed:
88 if (chore == null) {
89 throw new NullPointerException("chore cannot be null");
90 }
Why not have intialdelay be Chore too rather than pass it in? Because it a
one-off? Gets a bit awkward when you have to say this: "The initial delay
before the scheduled chore becomes active. Measured in
109 * same units as chore.getTimeUnit()" ... in other words,
the initial delay depends on Chore... bettter if if didnt'. This is a nit.
If all methods are synchronized, why do we need concurrent hash map?
You should probably tighten up your class and not let stuff like
public int getNumberOfChoresMissingStartTime() { out to anyone but unit tests.
TMI.
Check out how threads are named in hbase. We'll usually add the server name for
a prefix. Helps when you have many of them all running in the one JVM.
This should be internal detail?
public synchronized boolean requestCorePoolIncrease() {
Shut down the likes of t his till someone asks for it:
218 public void onChoreMadeStartTime(ScheduledChore
chore) {
219 // TODO: This is a placeholder for future extension... As it
currently stands,
Shut down access to this?
225 public synchronized void onChoreMissedStartTime(ScheduledChore chore)
{
Internal detail?
I'd say keep the debug stuff in but just make it log at trace level (and check
trace level is set).
FYI, don't need public qualifiers when defining an Interface... public
interface ChoreServicer { Its always public.
Do we need to reschedule? I don't remember this feature. It is nice but maybe
not needed?
Very nice tests.
Coming along real nice [~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)
