impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-3771: Expose kudu client timeout and set default
Date Fri, 04 Nov 2016 23:23:10 GMT
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-3771: Expose kudu client timeout and set default
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4849/5/be/src/exec/kudu-table-sink.h
File be/src/exec/kudu-table-sink.h:

Line 103:   /// This uses 'kudu::client::sp::shared_ptr' as that is the type expected by Kudu.
> this comment can probably be removed now that kudu has it's own namespace w
Done


http://gerrit.cloudera.org:8080/#/c/4849/5/testdata/workloads/functional-query/queries/QueryTest/kudu-timeouts-impalad.test
File testdata/workloads/functional-query/queries/QueryTest/kudu-timeouts-impalad.test:

Line 7: Unable to initialize the Kudu scan node
> will these always give the error? couldn't it possibly work with 1ms timeou
Hmm yes, I suppose this (and in the other file) could be flaky. I'm not sure there'll be any
way to _ensure_ that any of these operations take longer than 1ms.

I can think of these options:
1) take these tests out. not the end of the world but nice to have this coverage.
2) leave them in (and add comments), maybe it'll never be an issue in practice. I can remove/xfail
them later if necessary, or see if the Kudu folks have ideas about how to make it time out.

I guess i'd prefer to stick with #2 for now since it seems likely that all of these operations
will actually timeout (all metadata ops involving the master, constructing a response with
a bunch of strings to serialize, etc), and the coverage is nice. What do you think?


-- 
To view, visit http://gerrit.cloudera.org:8080/4849
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad95e8e38aad4f76d21bac6879db6c02b3c3e045
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message