You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Siddharth Seth <ss...@apache.org> on 2017/05/17 00:30:37 UTC
Review Request 59325: Cleanup of structures required when LLAP access
from external clients completes
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59325/
-----------------------------------------------------------
Review request for hive, Jason Dere and Sergey Shelukhin.
Bugs: HIVE-14052
https://issues.apache.org/jira/browse/HIVE-14052
Repository: hive-git
Description
-------
Cleanup of structures required when LLAP access from external clients completes
Diffs
-----
llap-common/src/gen/protobuf/gen-java/org/apache/hadoop/hive/llap/daemon/rpc/LlapDaemonProtocolProtos.java ece31ed
llap-common/src/protobuf/LlapDaemonProtocol.proto 3a3a2b8
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryInfo.java ce2f457
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java daeb555
llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorTestHelpers.java 27c426c
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 868eec7
Diff: https://reviews.apache.org/r/59325/diff/1/
Testing
-------
Thanks,
Siddharth Seth
Re: Review Request 59325: Cleanup of structures required when LLAP
access from external clients completes
Posted by Siddharth Seth <ss...@apache.org>.
> On May 17, 2017, 2:46 a.m., Jason Dere wrote:
> > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java
> > Lines 490 (patched)
> > <https://reviews.apache.org/r/59325/diff/1/?file=1722071#file1722071line509>
> >
> > Does the size() check need to happen within the lock - what if size() == 0, but a new fragment is added while we try to get the dagLock?
I don't think it makes a difference. Will move the check into the writeLock to be safe.
- Siddharth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59325/#review175192
-----------------------------------------------------------
On May 17, 2017, 12:30 a.m., Siddharth Seth wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59325/
> -----------------------------------------------------------
>
> (Updated May 17, 2017, 12:30 a.m.)
>
>
> Review request for hive, Jason Dere and Sergey Shelukhin.
>
>
> Bugs: HIVE-14052
> https://issues.apache.org/jira/browse/HIVE-14052
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Cleanup of structures required when LLAP access from external clients completes
>
>
> Diffs
> -----
>
> llap-common/src/gen/protobuf/gen-java/org/apache/hadoop/hive/llap/daemon/rpc/LlapDaemonProtocolProtos.java ece31ed
> llap-common/src/protobuf/LlapDaemonProtocol.proto 3a3a2b8
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryInfo.java ce2f457
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java daeb555
> llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorTestHelpers.java 27c426c
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 868eec7
>
>
> Diff: https://reviews.apache.org/r/59325/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Siddharth Seth
>
>
Re: Review Request 59325: Cleanup of structures required when LLAP
access from external clients completes
Posted by Jason Dere <jd...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59325/#review175192
-----------------------------------------------------------
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java
Lines 490 (patched)
<https://reviews.apache.org/r/59325/#comment248583>
Does the size() check need to happen within the lock - what if size() == 0, but a new fragment is added while we try to get the dagLock?
- Jason Dere
On May 17, 2017, 12:30 a.m., Siddharth Seth wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59325/
> -----------------------------------------------------------
>
> (Updated May 17, 2017, 12:30 a.m.)
>
>
> Review request for hive, Jason Dere and Sergey Shelukhin.
>
>
> Bugs: HIVE-14052
> https://issues.apache.org/jira/browse/HIVE-14052
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Cleanup of structures required when LLAP access from external clients completes
>
>
> Diffs
> -----
>
> llap-common/src/gen/protobuf/gen-java/org/apache/hadoop/hive/llap/daemon/rpc/LlapDaemonProtocolProtos.java ece31ed
> llap-common/src/protobuf/LlapDaemonProtocol.proto 3a3a2b8
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryInfo.java ce2f457
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java daeb555
> llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorTestHelpers.java 27c426c
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 868eec7
>
>
> Diff: https://reviews.apache.org/r/59325/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Siddharth Seth
>
>
Re: Review Request 59325: Cleanup of structures required when LLAP
access from external clients completes
Posted by Siddharth Seth <ss...@apache.org>.
> On May 17, 2017, 2:02 a.m., Sergey Shelukhin wrote:
> > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java
> > Lines 464 (patched)
> > <https://reviews.apache.org/r/59325/diff/1/?file=1722071#file1722071line483>
> >
> > will there be a callable created after every single fragment that finishes for the query with no other fragments running? Perhaps one callable should wait for the entire query and the fragments coming it should just push back the timestamp at which the query would time out and be cleaned?
> >
> > also what if readLock blocks all cleanup attempts?
>
> Siddharth Seth wrote:
> In terms of the callable - was trying to keep it simple. Can try making use of a single callable.
>
> Why would a readLock block all cleanup attempts? If I'm not mistaken, a readLock means something else is running - which would cause another cleanup to be scheduled.
Haven't made the Callable change here. Made small improvements to when cleanup runs (it will not create a new lock).
Will file a follow up for the single Callable. Think it adds a bit of code since we can't have a thread wait, so it'll really be a one callable optionally schedules the next callable etc.
- Siddharth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59325/#review175184
-----------------------------------------------------------
On May 17, 2017, 9:56 p.m., Siddharth Seth wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59325/
> -----------------------------------------------------------
>
> (Updated May 17, 2017, 9:56 p.m.)
>
>
> Review request for hive, Jason Dere and Sergey Shelukhin.
>
>
> Bugs: HIVE-14052
> https://issues.apache.org/jira/browse/HIVE-14052
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Cleanup of structures required when LLAP access from external clients completes
>
>
> Diffs
> -----
>
> llap-common/src/gen/protobuf/gen-java/org/apache/hadoop/hive/llap/daemon/rpc/LlapDaemonProtocolProtos.java ece31ed
> llap-common/src/protobuf/LlapDaemonProtocol.proto 3a3a2b8
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryInfo.java ce2f457
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java daeb555
> llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorTestHelpers.java 27c426c
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 868eec7
>
>
> Diff: https://reviews.apache.org/r/59325/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Siddharth Seth
>
>
Re: Review Request 59325: Cleanup of structures required when LLAP
access from external clients completes
Posted by Siddharth Seth <ss...@apache.org>.
> On May 17, 2017, 2:02 a.m., Sergey Shelukhin wrote:
> > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java
> > Lines 192 (patched)
> > <https://reviews.apache.org/r/59325/diff/1/?file=1722071#file1722071line192>
> >
> > external tasks cannot use shuffle? just checking
No. They have their own data transfer mechanism, which should be complete by the time a task completes.
> On May 17, 2017, 2:02 a.m., Sergey Shelukhin wrote:
> > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java
> > Lines 464 (patched)
> > <https://reviews.apache.org/r/59325/diff/1/?file=1722071#file1722071line483>
> >
> > will there be a callable created after every single fragment that finishes for the query with no other fragments running? Perhaps one callable should wait for the entire query and the fragments coming it should just push back the timestamp at which the query would time out and be cleaned?
> >
> > also what if readLock blocks all cleanup attempts?
In terms of the callable - was trying to keep it simple. Can try making use of a single callable.
Why would a readLock block all cleanup attempts? If I'm not mistaken, a readLock means something else is running - which would cause another cleanup to be scheduled.
- Siddharth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59325/#review175184
-----------------------------------------------------------
On May 17, 2017, 12:30 a.m., Siddharth Seth wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59325/
> -----------------------------------------------------------
>
> (Updated May 17, 2017, 12:30 a.m.)
>
>
> Review request for hive, Jason Dere and Sergey Shelukhin.
>
>
> Bugs: HIVE-14052
> https://issues.apache.org/jira/browse/HIVE-14052
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Cleanup of structures required when LLAP access from external clients completes
>
>
> Diffs
> -----
>
> llap-common/src/gen/protobuf/gen-java/org/apache/hadoop/hive/llap/daemon/rpc/LlapDaemonProtocolProtos.java ece31ed
> llap-common/src/protobuf/LlapDaemonProtocol.proto 3a3a2b8
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryInfo.java ce2f457
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java daeb555
> llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorTestHelpers.java 27c426c
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 868eec7
>
>
> Diff: https://reviews.apache.org/r/59325/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Siddharth Seth
>
>
Re: Review Request 59325: Cleanup of structures required when LLAP
access from external clients completes
Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59325/#review175184
-----------------------------------------------------------
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java
Lines 192 (patched)
<https://reviews.apache.org/r/59325/#comment248574>
external tasks cannot use shuffle? just checking
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java
Lines 464 (patched)
<https://reviews.apache.org/r/59325/#comment248582>
will there be a callable created after every single fragment that finishes for the query with no other fragments running? Perhaps one callable should wait for the entire query and the fragments coming it should just push back the timestamp at which the query would time out and be cleaned?
also what if readLock blocks all cleanup attempts?
- Sergey Shelukhin
On May 17, 2017, 12:30 a.m., Siddharth Seth wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59325/
> -----------------------------------------------------------
>
> (Updated May 17, 2017, 12:30 a.m.)
>
>
> Review request for hive, Jason Dere and Sergey Shelukhin.
>
>
> Bugs: HIVE-14052
> https://issues.apache.org/jira/browse/HIVE-14052
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Cleanup of structures required when LLAP access from external clients completes
>
>
> Diffs
> -----
>
> llap-common/src/gen/protobuf/gen-java/org/apache/hadoop/hive/llap/daemon/rpc/LlapDaemonProtocolProtos.java ece31ed
> llap-common/src/protobuf/LlapDaemonProtocol.proto 3a3a2b8
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryInfo.java ce2f457
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java daeb555
> llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorTestHelpers.java 27c426c
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 868eec7
>
>
> Diff: https://reviews.apache.org/r/59325/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Siddharth Seth
>
>
Re: Review Request 59325: Cleanup of structures required when LLAP
access from external clients completes
Posted by Siddharth Seth <ss...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59325/
-----------------------------------------------------------
(Updated May 17, 2017, 9:56 p.m.)
Review request for hive, Jason Dere and Sergey Shelukhin.
Changes
-------
Fixes review comments.
Adds one additional change to use a consistent "queryId". Earlier, RecordReader would pick up a different queryId from the rest of the system, which would geneate 2 log files. Only one of the two log files would get rotated.
Bugs: HIVE-14052
https://issues.apache.org/jira/browse/HIVE-14052
Repository: hive-git
Description
-------
Cleanup of structures required when LLAP access from external clients completes
Diffs (updated)
-----
llap-common/src/gen/protobuf/gen-java/org/apache/hadoop/hive/llap/daemon/rpc/LlapDaemonProtocolProtos.java ece31ed
llap-common/src/protobuf/LlapDaemonProtocol.proto 3a3a2b8
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryInfo.java ce2f457
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java daeb555
llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorTestHelpers.java 27c426c
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java 868eec7
Diff: https://reviews.apache.org/r/59325/diff/2/
Changes: https://reviews.apache.org/r/59325/diff/1-2/
Testing
-------
Thanks,
Siddharth Seth