[kudu-CR] KUDU-2249 give the TableRecordReader their own KuduClient to use.

2018-01-03 Thread David Ribeiro Alves (Code Review) 
David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8921 )

Change subject: KUDU-2249 give the TableRecordReader their own KuduClient to 
use.
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8921/1/java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java
File 
java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java:

http://gerrit.cloudera.org:8080/#/c/8921/1/java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java@253
PS1, Line 253:   private KuduClient getKuduClient() {
 :
 : String masterAddresses = conf.get(MASTER_ADDRESSES_KEY);
 : this.operationTimeoutMs = 
conf.getLong(OPERATION_TIMEOUT_MS_KEY,
 : AsyncKuduClient.DEFAULT_OPERATION_TIMEOUT_MS);
 : KuduClient kuduClient = new 
KuduClient.KuduClientBuilder(masterAddresses)
 : .defaultOperationTimeoutMs(operationTimeoutMs)
 : .build();
 : 
KuduTableMapReduceUtil.importCredentialsFromCurrentSubject(kuduClient);
 : return kuduClient;
 :   }
> Hi David,
Thanks for pointing out the discussion.
I'd still like to, at the very least, have a comment, on the commit message 
and/or on the code explaining: i) the rationale, in terms of how many new 
clients we can expect; ii) how this is not going to cause 10s or 100s of new 
clients to spawn, increasing the load on the master iii) or if it is, how there 
is no way to avoid it.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I24f45ee9253790c5348cabd0afe6c6a4b6d3f3d4
Gerrit-Change-Number: 8921
Gerrit-PatchSet: 1
Gerrit-Owner: Clemens Valiente 
Gerrit-Reviewer: Clemens Valiente 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 04 Jan 2018 04:16:09 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191 (4/n): HMS Thrift client fault handling

2018-01-03 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8494 )

Change subject: KUDU-2191 (4/n): HMS Thrift client fault handling
..


Patch Set 7:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/8494/7/src/kudu/hms/hms_client.h
File src/kudu/hms/hms_client.h:

http://gerrit.cloudera.org:8080/#/c/8494/7/src/kudu/hms/hms_client.h@51
PS7, Line 51: the
to


http://gerrit.cloudera.org:8080/#/c/8494/7/src/kudu/hms/hms_client.h@84
PS7, Line 84: explicit
No longer needed now that this is multi-arg?


http://gerrit.cloudera.org:8080/#/c/8494/7/src/kudu/hms/hms_client.h@85
PS7, Line 85:  MonoDelta send_timeout = 
MonoDelta::FromSeconds(60),
Would be more ergonomic to create an HmsClientOptions struct and stuff these 
(with documentation) in there.


http://gerrit.cloudera.org:8080/#/c/8494/7/src/kudu/hms/mini_hms.cc
File src/kudu/hms/mini_hms.cc:

http://gerrit.cloudera.org:8080/#/c/8494/7/src/kudu/hms/mini_hms.cc@111
PS7, Line 111: "-p", std::to_string(port_),
Was this change made so that you could use Start() to restart and have it use 
the same port as before? Might want to document that behavior.

Also, how do we prevent test flakiness caused by another application binding to 
our previously bound ephemeral port?


http://gerrit.cloudera.org:8080/#/c/8494/7/src/kudu/hms/mini_hms.cc@131
PS7, Line 131: WARN_NOT_OK
Why not RETURN_NOT_OK?


http://gerrit.cloudera.org:8080/#/c/8494/7/src/kudu/hms/mini_hms.cc@139
PS7, Line 139: KUDU
You can drop this prefix. In Continue() too.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic48bbb833bbae39b383ae239054b9710da3c746d
Gerrit-Change-Number: 8494
Gerrit-PatchSet: 7
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 04 Jan 2018 00:36:29 +
Gerrit-HasComments: Yes

[kudu-CR] Add 'kudu fs list' tool

2018-01-03 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8911 )

Change subject: Add 'kudu fs list' tool
..


Patch Set 6:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/8911/6/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/8911/6/src/kudu/tools/kudu-tool-test.cc@404
PS6, Line 404: const vector kFsModeRegexes = {
This should be updated.


http://gerrit.cloudera.org:8080/#/c/8911/6/src/kudu/tools/tool_action_fs.cc
File src/kudu/tools/tool_action_fs.cc:

http://gerrit.cloudera.org:8080/#/c/8911/6/src/kudu/tools/tool_action_fs.cc@396
PS6, Line 396:   switch (group) {
What happens if I add an entry to FieldGroup but forgot to update this switch? 
Do I get a compile error? A warning? Nothing?

If it's not a compile error, is there anything we should add here to guarantee 
good behavior? Like a default statement, a LOG(FATAL), or something like that?


http://gerrit.cloudera.org:8080/#/c/8911/6/src/kudu/tools/tool_action_fs.cc@415
PS6, Line 415:   static const Field kFieldVariants[] = {
Could you use the enum macros from gutil/casts.h to avoid this?


http://gerrit.cloudera.org:8080/#/c/8911/6/src/kudu/tools/tool_action_fs.cc@483
PS6, Line 483: // Returns rowset info for the field.
Worth logging the min/max keys of the rowset?


http://gerrit.cloudera.org:8080/#/c/8911/6/src/kudu/tools/tool_action_fs.cc@584
PS6, Line 584: CHECK_OK(fs_manager->OpenBlock(block, _block));
CHECK_OK seems wrong for a CLI tool; why not RETURN_NOT_OK and return an error 
from List if one of these fails?


http://gerrit.cloudera.org:8080/#/c/8911/6/src/kudu/tools/tool_action_fs.cc@620
PS6, Line 620: tablet_ids.emplace_back(FLAGS_tablet_id);
Do we need to ToLowerCase this?


http://gerrit.cloudera.org:8080/#/c/8911/6/src/kudu/tools/tool_action_fs.cc@633
PS6, Line 633: WARN_NOT_OK(TabletMetadata::Load(_manager, tablet_id, 
_metadata),
Why not RETURN_NOT_OK on this?


http://gerrit.cloudera.org:8080/#/c/8911/6/src/kudu/tools/tool_action_fs.cc@652
PS6, Line 652:   if (FLAGS_rowset_id > 0 && FLAGS_rowset_id != rowset.id()) 
{
Shouldn't this be FLAGS_rowset_id != -1 && FLAGS_rowset_id != rowset.id() ?


http://gerrit.cloudera.org:8080/#/c/8911/6/src/kudu/tools/tool_action_fs.cc@688
PS6, Line 688:   // TODO(dan): should orphaned blocks be included, perhaps 
behind a flag?
Perhaps, but this comment is slightly misplaced; orphaned blocks are a 
tablet-level thing, not a rowset-level thing, so the comment should be outside 
the inner (rowset_metadata) loop.


http://gerrit.cloudera.org:8080/#/c/8911/6/src/kudu/tools/tool_action_fs.cc@786
PS6, Line 786:"Possible values: table, 
table-id, tablet-id, partition, "
Seems like it might be easy to accidentally omit an entry; could we construct 
this list on-the-fly by iterating on the Field enum class?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f5a63e636d95e3ee55bb4955cece7f5d0b7532d
Gerrit-Change-Number: 8911
Gerrit-PatchSet: 6
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 04 Jan 2018 00:22:53 +
Gerrit-HasComments: Yes

[kudu-CR] periodic: fix a comment

2018-01-03 Thread Adar Dembo (Code Review) 
Adar Dembo has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8931 )

Change subject: periodic: fix a comment
..

periodic: fix a comment

Change-Id: Id04bff4f6505595f60f26abe88de58cdded4d9d0
Reviewed-on: http://gerrit.cloudera.org:8080/8931
Reviewed-by: Alexey Serbin 
Tested-by: Kudu Jenkins
---
M src/kudu/rpc/periodic.h
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Alexey Serbin: Looks good to me, approved
  Kudu Jenkins: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id04bff4f6505595f60f26abe88de58cdded4d9d0
Gerrit-Change-Number: 8931
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy

[kudu-CR] periodic: fix a comment

2018-01-03 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8931 )

Change subject: periodic: fix a comment
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id04bff4f6505595f60f26abe88de58cdded4d9d0
Gerrit-Change-Number: 8931
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Wed, 03 Jan 2018 20:09:27 +
Gerrit-HasComments: No

[kudu-CR] periodic: fix a comment

2018-01-03 Thread Adar Dembo (Code Review) 
Hello Mike Percy, Alexey Serbin,

I'd like you to do a code review. Please visit

http://gerrit.cloudera.org:8080/8931

to review the following change.


Change subject: periodic: fix a comment
..

periodic: fix a comment

Change-Id: Id04bff4f6505595f60f26abe88de58cdded4d9d0
---
M src/kudu/rpc/periodic.h
1 file changed, 1 insertion(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/8931/1
--
To view, visit http://gerrit.cloudera.org:8080/8931
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id04bff4f6505595f60f26abe88de58cdded4d9d0
Gerrit-Change-Number: 8931
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Mike Percy

[kudu-CR] KUDU-721: Support for Decimal type, Part 1 (Server, C++ Client)

2018-01-03 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8830 )

Change subject: KUDU-721: Support for Decimal type, Part 1 (Server, C++ Client)
..


Patch Set 6:

(12 comments)

It seems I took a quick look at PS1 some time ago.  Will re-visit this week.

http://gerrit.cloudera.org:8080/#/c/8830/5/src/kudu/client/schema.h
File src/kudu/client/schema.h:

http://gerrit.cloudera.org:8080/#/c/8830/5/src/kudu/client/schema.h@64
PS5, Line 64: public:
nit: alignment


http://gerrit.cloudera.org:8080/#/c/8830/1/src/kudu/client/schema.h
File src/kudu/client/schema.h:

http://gerrit.cloudera.org:8080/#/c/8830/1/src/kudu/client/schema.h@71
PS1, Line 71: : pre
I don't think this is needed unless we want to protect against list-initialized 
constructors.


http://gerrit.cloudera.org:8080/#/c/8830/1/src/kudu/client/schema.h@76
PS1, Line 76:   ret
drop


http://gerrit.cloudera.org:8080/#/c/8830/1/src/kudu/client/schema.h@81
PS1, Line 81:   ret
drop


http://gerrit.cloudera.org:8080/#/c/8830/1/src/kudu/client/schema.h@321
PS1, Line 321:   ///
Add the documented description for the precision parameter.


http://gerrit.cloudera.org:8080/#/c/8830/1/src/kudu/client/schema.h@328
PS1, Line 328:   ///
Add the documented description for the scale parameter.


http://gerrit.cloudera.org:8080/#/c/8830/5/src/kudu/common/common.proto
File src/kudu/common/common.proto:

http://gerrit.cloudera.org:8080/#/c/8830/5/src/kudu/common/common.proto@54
PS5, Line 54:   DECIMAL32 = 17;
nit: could you add a comment to explain why 15 and 16 are skipped?


http://gerrit.cloudera.org:8080/#/c/8830/5/src/kudu/common/decimal_value.h
File src/kudu/common/decimal_value.h:

http://gerrit.cloudera.org:8080/#/c/8830/5/src/kudu/common/decimal_value.h@32
PS5, Line 32:
nit: spacing


http://gerrit.cloudera.org:8080/#/c/8830/5/src/kudu/common/decimal_value.h@43
PS5, Line 43:
nit: by code guidelines, the indent should be 2 spaces.


http://gerrit.cloudera.org:8080/#/c/8830/1/src/kudu/common/schema.cc
File src/kudu/common/schema.cc:

http://gerrit.cloudera.org:8080/#/c/8830/1/src/kudu/common/schema.cc@50
PS1, Line 50:  DataType type) const {
nit: misaligned line for the second parameter


http://gerrit.cloudera.org:8080/#/c/8830/1/src/kudu/common/schema.cc@57
PS1, Line 57: true
Why true, not false?  If there is some specific reason, please add a comment 
about that.


http://gerrit.cloudera.org:8080/#/c/8830/1/src/kudu/common/schema.cc@125
PS1, Line 125:   return strings::Substitute("$0$1 $2",
nit: missed space



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3b06142f43c66973f36376bd2c88ca6f8d9f7632
Gerrit-Change-Number: 8830
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 03 Jan 2018 19:57:40 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-721: Support for Decimal type, Part 1 (Server, C++ Client)

2018-01-03 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8830 )

Change subject: KUDU-721: Support for Decimal type, Part 1 (Server, C++ Client)
..


Patch Set 6:

(4 comments)

I only looked at the public API changes.

http://gerrit.cloudera.org:8080/#/c/8830/6/src/kudu/client/schema.h
File src/kudu/client/schema.h:

http://gerrit.cloudera.org:8080/#/c/8830/6/src/kudu/client/schema.h@64
PS6, Line 64: public:
Nit: indentation


http://gerrit.cloudera.org:8080/#/c/8830/6/src/kudu/client/schema.h@70
PS6, Line 70:   explicit KuduColumnTypeAttributes(int32_t precision, int32_t 
scale)
The 'explicit' keyword is only needed to avoid implicit conversions with 
single-arg constructors, no?


http://gerrit.cloudera.org:8080/#/c/8830/6/src/kudu/client/schema.h@75
PS6, Line 75:   const int32_t precision() const {
These accessors are returning an int32_t 'copy', so what value does the 'const' 
keyword add (as in 'const int32_t ...')?


http://gerrit.cloudera.org:8080/#/c/8830/6/src/kudu/client/schema.h@84
PS6, Line 84:  private:
Do you anticipate KuduColumnTypeAttributes evolving for other use cases in the 
future? If so, you should PIMPL the class: define a private Data nested class, 
store precision/scale within it, and have the public class merely store an 
allocated pointer to the nested class.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3b06142f43c66973f36376bd2c88ca6f8d9f7632
Gerrit-Change-Number: 8830
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Wed, 03 Jan 2018 19:18:24 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

2018-01-03 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8757 )

Change subject: KUDU-2216. Post process gtest generated xml to include the 
output from the *.txt files
..


Patch Set 12:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8757/12/build-support/jenkins/add_std_out_to_junit.py
File build-support/jenkins/add_std_out_to_junit.py:

http://gerrit.cloudera.org:8080/#/c/8757/12/build-support/jenkins/add_std_out_to_junit.py@68
PS12, Line 68:  Globs for *.gz ands *. xml logs in log_location, then sorts 
the logs
I'm missing a ton of context, but why not run this script out of run-test.sh 
instead of build-and-test.sh? That way it'll run once per test and the precise 
gz/xml file pair will be known, right?


http://gerrit.cloudera.org:8080/#/c/8757/12/build-support/jenkins/add_std_out_to_junit.py@69
PS12, Line 69:  I
Comment style nit: you'll find the pronouns "we" and "us" more commonly used 
than "I" and "me".



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
Gerrit-Change-Number: 8757
Gerrit-PatchSet: 12
Gerrit-Owner: Edward Fancher 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Edward Fancher 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 03 Jan 2018 19:07:51 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2229. consensus: Leader should not start FD

2018-01-03 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8711 )

Change subject: KUDU-2229. consensus: Leader should not start FD
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8711/4/src/kudu/rpc/periodic.h
File src/kudu/rpc/periodic.h:

http://gerrit.cloudera.org:8080/#/c/8711/4/src/kudu/rpc/periodic.h@144
PS4, Line 144:   // Returns true iff the failure detected has been started.
FWIW, this comment is wrong: it refers to failure detection rather than to this 
generic periodic timer system.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2ec9c5499e8e4c1659333bd53dd2d7f6dae81f5
Gerrit-Change-Number: 8711
Gerrit-PatchSet: 4
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Wed, 03 Jan 2018 18:51:52 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2249 give the TableRecordReader their own KuduClient to use.

2018-01-03 Thread Clemens Valiente (Code Review) 
Clemens Valiente has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8921 )

Change subject: KUDU-2249 give the TableRecordReader their own KuduClient to 
use.
..


Patch Set 1:

(1 comment)

Hi David,
I discussed this on the dev mailing list here:
http://mail-archives.apache.org/mod_mbox/kudu-dev/201712.mbox/%3CAM0PR0502MB405140DD23048A17522BB55C9C070%40AM0PR0502MB4051.eurprd05.prod.outlook.com%3E

The problem is the getInputSplit() closing the client that the Reader want to 
use.

There's no real clean way of sharing the client and closing it properly. Due to 
the mapreduce architecture, each tablet usually will read from a separate Map 
Container and thus need its own client anyway. This just fixes a bug in the 
rare scenario that the getinputsplit and record reader are executed in one 
container.

http://gerrit.cloudera.org:8080/#/c/8921/1/java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java
File 
java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java:

http://gerrit.cloudera.org:8080/#/c/8921/1/java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java@253
PS1, Line 253:   private KuduClient getKuduClient() {
 :
 : String masterAddresses = conf.get(MASTER_ADDRESSES_KEY);
 : this.operationTimeoutMs = 
conf.getLong(OPERATION_TIMEOUT_MS_KEY,
 : AsyncKuduClient.DEFAULT_OPERATION_TIMEOUT_MS);
 : KuduClient kuduClient = new 
KuduClient.KuduClientBuilder(masterAddresses)
 : .defaultOperationTimeoutMs(operationTimeoutMs)
 : .build();
 : 
KuduTableMapReduceUtil.importCredentialsFromCurrentSubject(kuduClient);
 : return kuduClient;
 :   }
> Kudu clients cache state internally, like tablet locations. Changing this t
Hi David,
I discussed this on the dev mailing list here:
http://mail-archives.apache.org/mod_mbox/kudu-dev/201712.mbox/%3CAM0PR0502MB405140DD23048A17522BB55C9C070%40AM0PR0502MB4051.eurprd05.prod.outlook.com%3E

The problem is the getInputSplit() closing the client that the Reader want to 
use.

There's no real clean way of sharing the client and closing it properly. Due to 
the mapreduce architecture, each tablet usually will read from a separate Map 
Container and thus need its own client anyway. This just fixes a bug in the 
rare scenario that the getinputsplit and record reader are executed in one 
container.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I24f45ee9253790c5348cabd0afe6c6a4b6d3f3d4
Gerrit-Change-Number: 8921
Gerrit-PatchSet: 1
Gerrit-Owner: Clemens Valiente 
Gerrit-Reviewer: Clemens Valiente 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 03 Jan 2018 08:39:45 +
Gerrit-HasComments: Yes