[
https://issues.apache.org/jira/browse/HIVE-16104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15904326#comment-15904326
]
Sergey Shelukhin edited comment on HIVE-16104 at 3/10/17 3:15 AM:
------------------------------------------------------------------
See
https://docs.oracle.com/javase/7/docs/api/java/lang/System.html#nanoTime%28%29,
it can be negative. Anyway there's no reason to have a magic value when it can
be null.
{quote}
That's a style choice. There's nothing wrong with using an executor with a
single thread. If anything, this should have been
Executors.newSingleThreadExecutor
{quote}
That's complexity for no reason.. so not really a style choice.
{quote}
Think we're better off using the mega lock in the scheduler for the wait - that
way it gets interrupted if some other task completes, another task asks to be
scheduled, etc. (Doesn't need to wait for the specific fragment to end)
{quote}
Sorry didn't get that. You mean for the waiting for completion? Wouldn't that
block other ops like adding tasks?
{quote}
Using isComplete as the wait mechanism has a race when the setIsCompleted()
invocation happens in TaskRunnerCallable.
{quote}
What is the race? The wait is on the lock and so both wait and notify have to
execute when the lock is taken. isComplete is also only examined under the same
lock. So either the checker sees isComplete=true (after someone has set it and
ran notify) and doesn't wait, or it sees false, so whoever notifies-All cannot
notify before the checker releases the lock by going into wait, cause he needs
the lock to notify; hence the waiting thread will get notified. Also isComplete
is never unset, so that makes it even less problematic cause noone will set it
to false.
was (Author: sershe):
See
https://docs.oracle.com/javase/7/docs/api/java/lang/System.html#nanoTime%28%29,
it can be negative. Anyway there's no reason to have a magic value when it can
be null.
{quote}
That's a style choice. There's nothing wrong with using an executor with a
single thread. If anything, this should have been
Executors.newSingleThreadExecutor
{quote}
That's complexity for no reason.. so not really a style choice.
{quote}
Think we're better off using the mega lock in the scheduler for the wait - that
way it gets interrupted if some other task completes, another task asks to be
scheduled, etc. (Doesn't need to wait for the specific fragment to end)
{quote}
Sorry didn't get that. You mean for the waiting for completion? Wouldn't that
block other ops like adding tasks?
{quote}
Using isComplete as the wait mechanism has a race when the setIsCompleted()
invocation happens in TaskRunnerCallable.
{quote}
What is the race? The wait is on the lock and so both wait and notify have to
execute when the lock is taken. isComplete is also only examined under the same
lock. So either the checker sees isComplete=true (after someone has set it and
ran notify) and doesn't wait, or it sees false, so whoever notifies-All cannot
notify before the checker releases the lock, cause he needs the lock to notify.
Even if he could, at worst checker would wait needlessly and return value of
isComplete. Also isComplete is never unset, so that makes it even less
problematic cause noone will set it to false.
> LLAP: preemption may be too aggressive if the pre-empted task doesn't die
> immediately
> -------------------------------------------------------------------------------------
>
> Key: HIVE-16104
> URL: https://issues.apache.org/jira/browse/HIVE-16104
> Project: Hive
> Issue Type: Bug
> Reporter: Sergey Shelukhin
> Assignee: Sergey Shelukhin
> Attachments: HIVE-16104.01.patch, HIVE-16104.02.patch,
> HIVE-16104.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
