[kudu-CR] [fs] fix build on OS X

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded a new change for review.

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

Change subject: [fs] fix build on OS X
..

[fs] fix build on OS X

Added kudu_fs dependency for kudu_fs_test_util library.

Prior to this patch, the following error was reported if trying to build
on OS X:

Undefined symbols for architecture x86_64:
  "kudu::fs::LogBlockManager::kContainerDataFileSuffix", referenced from:
  kudu::fs::LBMCorruptor::Init() in log_block_manager-test-util.cc.o
  kudu::fs::LBMCorruptor::CreateIncompleteContainer()
  in log_block_manager-test-util.cc.o
  "kudu::fs::LogBlockManager::kContainerMetadataFileSuffix", referenced from:
  kudu::fs::LBMCorruptor::Init() in log_block_manager-test-util.cc.o
  kudu::fs::LBMCorruptor::CreateIncompleteContainer()
  in log_block_manager-test-util.cc.o
  "fLU64::FLAGS_log_container_max_size", referenced from:
  kudu::fs::LBMCorruptor::Init() in log_block_manager-test-util.cc.o
  "kudu::BlockId::CopyToPB(kudu::BlockIdPB*) const", referenced from:
  kudu::fs::LBMCorruptor::AddMalformedRecordToContainer()
  in log_block_manager-test-util.cc.o
  kudu::fs::LBMCorruptor::AddMisalignedBlockToContainer()
  in log_block_manager-test-util.cc.o
  kudu::fs::LBMCorruptor::AddPartialRecordToContainer()
  in log_block_manager-test-util.cc.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [lib/libkudu_fs_test_util.dylib] Error 1

Change-Id: I60e16871818bb8994a2ecc59628ba22633ece3ba
---
M src/kudu/fs/CMakeLists.txt
1 file changed, 1 insertion(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I60e16871818bb8994a2ecc59628ba22633ece3ba
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 

[kudu-CR] WIP: KUDU-463. Add checksumming to cfile

[Grant Henke (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

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

to look at the new patch set (#12).

Change subject: WIP: KUDU-463. Add checksumming to cfile
..

WIP: KUDU-463. Add checksumming to cfile

Adds optional checksumming and validation to cfile blocks.

Introduces 2 flags to control cfile checksumming:
- cfile_write_checksums (default false)
- cfile_verify_checksums (default true)

cfile_write_checksums is used in the CFileWriter to enable computing and
appending Crc32 checksums to the end of each cfile block, header and footer

cfile_write_checksums is defaulted to false to ensure upgrades don't
immediately result in performance degredation and incompatible data on
downgrade. It can and likely should be defaulted to true in a later release.

When cfile_write_checksums is set to true, the existing incompatible_features
bitset in the cfile footer is used. A "checksum" bit is set to ensure a clear
error message when older versions of Kudu try to read the file. If checksums
are not written the incompatible_features "checksum" bit is not set.

cfile_verify_checksums is used in the CFileReader to enable validating the
data against the written checksum. cfile_verify_checksums is defaulted to
true since validation only occurs if checksums are written. Any data that
was written before checksumming was an option or when cfile_write_checksums
was false will not be verified.

Change-Id: I6756834cd7f27af258797a3654a95244abeb0976
---
M docs/design-docs/cfile.md
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/cfile_reader.h
M src/kudu/cfile/cfile_util.h
M src/kudu/cfile/cfile_writer.cc
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/block_manager.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/fs-test-util.h
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/crc-test.cc
M src/kudu/util/crc.cc
M src/kudu/util/crc.h
M src/kudu/util/env-test.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/file_cache.cc
18 files changed, 724 insertions(+), 63 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/6630/12
-- 
To view, visit http://gerrit.cloudera.org:8080/6630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6756834cd7f27af258797a3654a95244abeb0976
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] external mini cluster: spawn perf record for each daemon during Start()

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: external mini cluster: spawn perf record for each daemon during 
Start()
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6742/1/src/kudu/integration-tests/external_mini_cluster.cc
File src/kudu/integration-tests/external_mini_cluster.cc:

PS1, Line 270:   if (FLAGS_perf_record) {
 : opts.perf_record_filename =
 : Substitute("$0/perf-$1.data", opts.log_dir, daemon_id);
 :   }
> nit: does it matter that the flag is set? i.e. can't you just set the filen
Look at L718: we use the length of the filename to decide whether to spawn perf 
or not.

It's a little unintuitive but it's better than more state.


PS1, Line 720: "perf",
 :   "record",
 :   "--call-graph",
 :   "fp",
 :   "-o",
> it seems we're now calling this in multiple places. worth a util method tha
Eh. It's only two places, and full_stack-insert-scan-test makes the 
--call-graph part optional. So I think I'll pass.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I92079f616648788b461d550057b8e23bc9174b71
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1957: Clarify web UI redaction in --redact flag help

[Hao Hao (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

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

to look at the new patch set (#3).

Change subject: KUDU-1957: Clarify web UI redaction in --redact flag help
..

KUDU-1957: Clarify web UI redaction in --redact flag help

Currently,row data in web UI will be redacted if --redact
flag is set to 'log'. It would be better to have a seperate
option to enable web UI redaction.

This adds a new option 'web' to specify web UI redaction via
--redact flag. And updates help description properly.

Change-Id: I32fe0c4e65f22d6f9f3d188018b119f07300e26f
---
M src/kudu/cfile/binary_prefix_block.cc
M src/kudu/cfile/block_compression.cc
M src/kudu/cfile/bloomfile.cc
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/cfile_writer.cc
M src/kudu/client/batcher.cc
M src/kudu/client/session-internal.cc
M src/kudu/common/encoded_key.cc
M src/kudu/common/key_encoder.h
M src/kudu/common/partition.cc
M src/kudu/common/row_changelist.cc
M src/kudu/common/types.cc
M src/kudu/consensus/log_util.cc
M src/kudu/rpc/serialization.cc
M src/kudu/rpc/transfer.cc
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/concurrent_btree.h
M src/kudu/tablet/delta_key.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/lock_manager.cc
M src/kudu/tablet/rowset_info.cc
M src/kudu/util/compression/compression_codec.cc
M src/kudu/util/flags.cc
M src/kudu/util/hexdump.cc
M src/kudu/util/jsonwriter-test.cc
M src/kudu/util/jsonwriter.cc
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/pb_util.cc
M src/kudu/util/slice.cc
31 files changed, 113 insertions(+), 93 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/55/6755/3
-- 
To view, visit http://gerrit.cloudera.org:8080/6755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I32fe0c4e65f22d6f9f3d188018b119f07300e26f
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] log block manager: more container accounting

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: log block manager: more container accounting
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6716/3/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

Line 802: LOG(WARNING) << Substitute(
> i'm wondering why this isn't FATAL
I saw this trigger from time to time in 
LogBlockManagerTest.TestMisalignedBlocksFuzz, but I can't remember exactly why 
it happened (I can't reproduce it now). I think it was due to some combination 
of misaligned blocks and zero-length blocks.

Nevertheless, it's also defensive programming should CREATE records show up out 
of order in the container metadata for some reason.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1907db2a74bcc074ecf882f8a758c3989431e267
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1957: Clarify web UI redaction in --redact flag help

[Hao Hao (Code Review)]

Hao Hao has uploaded a new patch set (#2).

Change subject: KUDU-1957: Clarify web UI redaction in --redact flag help
..

KUDU-1957: Clarify web UI redaction in --redact flag help

Currently,row data in web UI will be redacted if --redact
flag is set to 'log'. It would be better to have a seperate
option to enable web UI redaction.

This adds a new option 'web' to specify web UI redaction via
--redact flag. And updates help description properly.

Change-Id: I32fe0c4e65f22d6f9f3d188018b119f07300e26f
---
M src/kudu/cfile/binary_prefix_block.cc
M src/kudu/cfile/block_compression.cc
M src/kudu/cfile/bloomfile.cc
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/cfile_writer.cc
M src/kudu/client/batcher.cc
M src/kudu/client/session-internal.cc
M src/kudu/common/encoded_key.cc
M src/kudu/common/key_encoder.h
M src/kudu/common/partition.cc
M src/kudu/common/row_changelist.cc
M src/kudu/common/types.cc
M src/kudu/consensus/log_util.cc
M src/kudu/rpc/serialization.cc
M src/kudu/rpc/transfer.cc
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/concurrent_btree.h
M src/kudu/tablet/delta_key.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/lock_manager.cc
M src/kudu/tablet/rowset_info.cc
M src/kudu/util/compression/compression_codec.cc
M src/kudu/util/flags.cc
M src/kudu/util/hexdump.cc
M src/kudu/util/jsonwriter-test.cc
M src/kudu/util/jsonwriter.cc
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/pb_util.cc
M src/kudu/util/slice.cc
31 files changed, 114 insertions(+), 94 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/55/6755/2
-- 
To view, visit http://gerrit.cloudera.org:8080/6755
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I32fe0c4e65f22d6f9f3d188018b119f07300e26f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-1978: avoid corruption when deleting misaligned blocks

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: KUDU-1978: avoid corruption when deleting misaligned blocks
..


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/6715/3/src/kudu/fs/log_block_manager-test-util.cc
File src/kudu/fs/log_block_manager-test-util.cc:

PS3, Line 288: id
> maybe say stringified id? at first i expected to just have the blockid in b
Done


PS3, Line 295: uint64_t block_length = rand_.Uniform(fs_block_size * 4);
 :   block_length -= block_length % block_id_str_len;
 :   uint8_t data[block_length];
 :   for (int i = 0; i < ARRAYSIZE(data); i += block_id_str_len) {
 : memcpy([i], block_id_str.data(), block_id_str_len);
 :   }
> it's ok if this writes a 0 sized block, right?
Yes, as long as the data file is fully preallocated up to the previous block 
(see length_beyond_eof).


http://gerrit.cloudera.org:8080/#/c/6715/3/src/kudu/fs/log_block_manager-test-util.h
File src/kudu/fs/log_block_manager-test-util.h:

PS3, Line 66: sequences of its size
> of its block id?
Whoops, that was from when I was writing the size. Done.


http://gerrit.cloudera.org:8080/#/c/6715/3/src/kudu/fs/log_block_manager-test.cc
File src/kudu/fs/log_block_manager-test.cc:

PS3, Line 782: Need to reopen the block manager after each corruption.
> why?
Clarified in the comment.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ed6e349ab10d8d04722cd7ef99e7a06554f51f1
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1957 Clarify web UI redaction in --redact flag help

[Hao Hao (Code Review)]

Hao Hao has uploaded a new change for review.

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

Change subject: KUDU-1957 Clarify web UI redaction in --redact flag help
..

KUDU-1957 Clarify web UI redaction in --redact flag help

Currently,row data in web UI will be redacted if --redact
flag is set to 'log'. It would be better to have a seperate
option to enable web UI redaction.

This adds a new option 'web' to specify web UI redaction via
--redact flag. And updates help description properly.

Change-Id: I32fe0c4e65f22d6f9f3d188018b119f07300e26f
---
M src/kudu/cfile/binary_prefix_block.cc
M src/kudu/cfile/block_compression.cc
M src/kudu/cfile/bloomfile.cc
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/cfile_writer.cc
M src/kudu/client/batcher.cc
M src/kudu/client/session-internal.cc
M src/kudu/common/encoded_key.cc
M src/kudu/common/key_encoder.h
M src/kudu/common/partition.cc
M src/kudu/common/row_changelist.cc
M src/kudu/common/types.cc
M src/kudu/consensus/log_util.cc
M src/kudu/rpc/serialization.cc
M src/kudu/rpc/transfer.cc
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/concurrent_btree.h
M src/kudu/tablet/delta_key.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/lock_manager.cc
M src/kudu/tablet/rowset_info.cc
M src/kudu/util/compression/compression_codec.cc
M src/kudu/util/flags.cc
M src/kudu/util/hexdump.cc
M src/kudu/util/jsonwriter-test.cc
M src/kudu/util/jsonwriter.cc
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/pb_util.cc
M src/kudu/util/slice.cc
31 files changed, 103 insertions(+), 88 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I32fe0c4e65f22d6f9f3d188018b119f07300e26f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 

[kudu-CR] thirdparty: fix typo in SPARSEHASH PATCHLEVEL

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: thirdparty: fix typo in SPARSEHASH_PATCHLEVEL
..


Patch Set 1: Verified+1

Failures were totally unrelated.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icbee25d01cdc16efdcadb7d897b134a7a3df2a43
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] thirdparty: fix typo in SPARSEHASH PATCHLEVEL

[Adar Dembo (Code Review)]

Adar Dembo has submitted this change and it was merged.

Change subject: thirdparty: fix typo in SPARSEHASH_PATCHLEVEL
..


thirdparty: fix typo in SPARSEHASH_PATCHLEVEL

I briefly experimented with using "set -o nounset" to prevent these problems
in the future, but we make use of undefined variables too much for it to be
an easy addition.

Change-Id: Icbee25d01cdc16efdcadb7d897b134a7a3df2a43
Reviewed-on: http://gerrit.cloudera.org:8080/6754
Reviewed-by: Dan Burkert 
Tested-by: Adar Dembo 
---
M thirdparty/download-thirdparty.sh
1 file changed, 2 insertions(+), 2 deletions(-)

Approvals:
  Dan Burkert: Looks good to me, approved
  Adar Dembo: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Icbee25d01cdc16efdcadb7d897b134a7a3df2a43
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] subprocess: remove progname argument

[Adar Dembo (Code Review)]

Adar Dembo has submitted this change and it was merged.

Change subject: subprocess: remove progname argument
..


subprocess: remove progname argument

When constructing a subprocess, 'argv[0]' always duplicates 'program', and
is typically BaseName()'d as well. Let's just codify that in the subprocess
code so that callers need not worry about it.

Change-Id: I42d3cb551c350d8f10308035084a8807a1ae240b
Reviewed-on: http://gerrit.cloudera.org:8080/6740
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/benchmarks/tpch/tpch_real_world.cc
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/full_stack-insert-scan-test.cc
M src/kudu/rpc/rpc_stub-test.cc
M src/kudu/security/test/mini_kdc.cc
M src/kudu/util/pstack_watcher.cc
M src/kudu/util/pstack_watcher.h
M src/kudu/util/subprocess-test.cc
M src/kudu/util/subprocess.cc
M src/kudu/util/subprocess.h
10 files changed, 33 insertions(+), 46 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I42d3cb551c350d8f10308035084a8807a1ae240b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] log block manager: corruptor test utility

[Adar Dembo (Code Review)]

Adar Dembo has submitted this change and it was merged.

Change subject: log block manager: corruptor test utility
..


log block manager: corruptor test utility

This patch introduces the LBMCorruptor, a test-only class for adding various
inconsistencies to the LBM's on-disk structures. The bulk of the patch is a
set of new tests that exercise the corruptor and then verify the results via
the generated FsReport.

Additionally, block_manager-stress-test has been modified to inject
non-fatal inconsistencies via the LBMCorruptor in between passes.

Change-Id: I197b693d5a3c1a909e91b772ebfb31e50bae488b
Reviewed-on: http://gerrit.cloudera.org:8080/6582
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
Reviewed-by: David Ribeiro Alves 
---
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/block_manager-stress-test.cc
A src/kudu/fs/log_block_manager-test-util.cc
A src/kudu/fs/log_block_manager-test-util.h
M src/kudu/fs/log_block_manager-test.cc
5 files changed, 857 insertions(+), 39 deletions(-)

Approvals:
  David Ribeiro Alves: Looks good to me, approved
  Todd Lipcon: Looks good to me, but someone else must approve
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I197b693d5a3c1a909e91b772ebfb31e50bae488b
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] fs: generate report during Open

[Adar Dembo (Code Review)]

Adar Dembo has submitted this change and it was merged.

Change subject: fs: generate report during Open
..


fs: generate report during Open

This patch modifies the FsManager and block managers to produce a filesystem
report when opened. The report includes various filesystem statistics as
well as all inconsistencies found and repaired.

The heart of the patch is the FsReport nested data structure. Originally
implemented as a protobuf to take advantage of DebugString(), I found it to
be a poor fit once I began customizing the log output, so I rewrote it in
its current form. The report includes fs-wide statistics as well as optional
consistency checks that may or may not be performed by the block managers.
Reports can be merged; the LBM takes advantage of this as it processes data
directories in parallel. The consistency checks have structure, so as to
simplify testing and ToString() customization. A sample report can be found
at the end of this commit message.

Thanks to the level of detail in the FsReport, the LBM now separates the act
of finding inconsistencies from repairing them. This makes it much easier to
enforce that repairs aren't done on a read-only filesystem, or if there were
any fatal and irreparable errors.

Thorough testing of the inconsistencies in the report is deferred to a
different patch as this one was already quite large.

Other changes of note:
- The LBM treats data files without matching metadata files as incomplete
  containers; previously only metadata files without matching data files
  were treated in this way.
- The LBM maps all containers by name so they can be looked up during the
  repair process; previously they were stored in a vector.
- The checks performed by "kudu fs check" are in FsReport. I think this
  makes sense, and it's an acknowledgement that they should be performed by
  the block manager in the future. The code in tool_action_fs.cc was
  restructured to write its findings into the report before logging it.

Block manager report

1 data directories: 
/tmp/kudutest-1000/block_manager-stress-test.BlockManagerStressTest_1.StressTest.1491963221016069-10059
Total live blocks: 47
Total live bytes: 221524
Total live bytes (after alignment): 323582
Total number of LBM containers: 53 (21 full)
Did not check for missing blocks
Did not check for orphaned blocks
Total full LBM containers with extra space: 0 (0 repaired)
Total full LBM container extra space in bytes: 0 (0 repaired)
Total incomplete LBM containers: 3 (3 repaired)
Misaligned block in container 
/tmp/kudutest-1000/block_manager-stress-test.BlockManagerStressTest_1.StressTest.1491963221016069-10059/038a826c27db4e6da8237ce79853ece2:
 block_id {
  id: 9225972546088407965
}
op_type: CREATE
timestamp_us: 0
offset: 1052673
length: 16384

Misaligned block in container 
/tmp/kudutest-1000/block_manager-stress-test.BlockManagerStressTest_1.StressTest.1491963221016069-10059/0612d7730bdc4cfd83afbaed271d5289:
 block_id {
  id: 3148617980286357277
}
op_type: CREATE
timestamp_us: 0
offset: 1048577
length: 16384

Total LBM partial records: 7 (7 repaired)

Change-Id: Idcc1d512e2bed103c8d41623a63b4d071532b35e
Reviewed-on: http://gerrit.cloudera.org:8080/6581
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/block_manager.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/file_block_manager.h
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
A src/kudu/fs/fs_report.cc
A src/kudu/fs/fs_report.h
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
M src/kudu/server/server_base.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/util/pb_util.cc
17 files changed, 1,104 insertions(+), 217 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Idcc1d512e2bed103c8d41623a63b4d071532b35e
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] subprocess: add ScopedSubprocess

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: subprocess: add ScopedSubprocess
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/6741/1/src/kudu/util/subprocess.h
File src/kudu/util/subprocess.h:

Line 185:   ~ScopedSubprocess() {
> nit: given this is a pretty heavy-weight function, i think it's better to d
Done


Line 190: WARN_NOT_OK(p_.Kill(signum_), "Could not kill subprocess");
> would be useful to include argv[0] or something in these warning messages h
Done


Line 191: Status s = p_.Wait();
> should there be any kind of timeout and fallback to kill -9?
Done


PS1, Line 193: WARNING
> error here and below
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf31bd4ea6de2917521a9852714fb33cfbec1f61
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] thirdparty: fix typo in SPARSEHASH PATCHLEVEL

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: thirdparty: fix typo in SPARSEHASH_PATCHLEVEL
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icbee25d01cdc16efdcadb7d897b134a7a3df2a43
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] thirdparty: fix typo in SPARSEHASH PATCHLEVEL

[Adar Dembo (Code Review)]

Hello Dan Burkert, Todd Lipcon,

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

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

to review the following change.

Change subject: thirdparty: fix typo in SPARSEHASH_PATCHLEVEL
..

thirdparty: fix typo in SPARSEHASH_PATCHLEVEL

I briefly experimented with using "set -o nounset" to prevent these problems
in the future, but we make use of undefined variables too much for it to be
an easy addition.

Change-Id: Icbee25d01cdc16efdcadb7d897b134a7a3df2a43
---
M thirdparty/download-thirdparty.sh
1 file changed, 2 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Icbee25d01cdc16efdcadb7d897b134a7a3df2a43
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Persistent cache support for NVM

[Sarah Jelinek (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

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

to look at the new patch set (#25).

Change subject: Persistent cache support for NVM
..

Persistent cache support for NVM

Change-Id: Id72570ead662670bf42175756a18ae08d7cd0a07
---
M build-support/run-test.sh
M src/kudu/cfile/block_cache.h
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/cfile_reader.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/cache-test.cc
M src/kudu/util/cache.cc
M src/kudu/util/cache.h
A src/kudu/util/nvm_cache-test.cc
M src/kudu/util/nvm_cache.cc
M src/kudu/util/nvm_cache.h
11 files changed, 862 insertions(+), 223 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/2593/25
-- 
To view, visit http://gerrit.cloudera.org:8080/2593
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id72570ead662670bf42175756a18ae08d7cd0a07
Gerrit-PatchSet: 25
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sarah Jelinek 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sarah Jelinek 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] external mini cluster: spawn perf record for each daemon during Start()

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: external mini cluster: spawn perf record for each daemon during 
Start()
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6742/1/src/kudu/integration-tests/external_mini_cluster.cc
File src/kudu/integration-tests/external_mini_cluster.cc:

PS1, Line 270:   if (FLAGS_perf_record) {
 : opts.perf_record_filename =
 : Substitute("$0/perf-$1.data", opts.log_dir, daemon_id);
 :   }
nit: does it matter that the flag is set? i.e. can't you just set the filename 
always? 3LOC less...


PS1, Line 720: "perf",
 :   "record",
 :   "--call-graph",
 :   "fp",
 :   "-o",
it seems we're now calling this in multiple places. worth a util method that 
just takes the pid?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I92079f616648788b461d550057b8e23bc9174b71
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR](gh-pages) FAQ refresh

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: FAQ refresh
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idd8a79e3067bedbef71b0dcf9921be673e9dfaa0
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Ambreen Kazi 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [KUDU-754] add an environment variable for kudu client debugging to stderr

[William Li (Code Review)]

William Li has posted comments on this change.

Change subject: [KUDU-754] add an environment variable for kudu client 
debugging to stderr
..


Patch Set 2:

(12 comments)

new patch has been submitted.

http://gerrit.cloudera.org:8080/#/c/6736/1//COMMIT_MSG
Commit Message:

Line 9: Read environment variable "KUDU_CLIENT_VERBOSE" to get verbose level.
> nit: can you re-wrap and format this into full sentences with appropriate c
Done


http://gerrit.cloudera.org:8080/#/c/6736/1/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:

Line 5049: 
> nit: whitespace here and below
Done


Line 5050: // Test that, log verbose level can be set through environment 
varialble
> nit: please wrap (here and a few lines down as well)
Done


Line 5053:   FLAGS_v = 0;
> no need for a separate variable here. I think it's clear what you're doing 
Done


Line 5059: 
> I think it's fine to just use '5' here literal instead of std::atoi() it ab
Done


http://gerrit.cloudera.org:8080/#/c/6736/1/src/kudu/client/client.cc
File src/kudu/client/client.cc:

PS1, Line 148: etClie
> this isn't a constant, so it should just be 'int level'
Done


PS1, Line 149: int32_t level = 0
> const char*?
Done


Line 151:   if (env_verbose_level != NULL) {
> style: { goes on the same line as 'if'. Same below.
Done


Line 152:  if (safe_strto32(env_verbose_level, ) && (level >=0)) {
> use safe_strto32() from numbers.h instead, so you can check validity. Maybe
Done


PS1, Line 153: el(level);
> why <= 6? Is there some limit on the vlog level as far as glog is concerned
I was reading the comments from client.h at line 110 paragraph: ".. the highest 
verbosity level used in Kudu is 6, ". I leave it open for the upper bound 
for now. If later you want to pick a big enough number to just set a 
limitation, we can do that too.


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

PS1, Line 125: /// Set signal number to use internally.
 : ///
> given this is called by the client during initialization, I don't think it'
Done


Line 129: /// this advanced API can help workaround conflicts.
> This should probably be declared extern in the header, and defined in the .
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: William Li 
Gerrit-HasComments: Yes

[kudu-CR] subprocess: add ScopedSubprocess

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: subprocess: add ScopedSubprocess
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6741/1/src/kudu/util/subprocess.h
File src/kudu/util/subprocess.h:

PS1, Line 193: WARNING
error here and below


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf31bd4ea6de2917521a9852714fb33cfbec1f61
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] log block manager: corruptor test utility

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: log block manager: corruptor test utility
..


Patch Set 11: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I197b693d5a3c1a909e91b772ebfb31e50bae488b
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1978: avoid corruption when deleting misaligned blocks

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1978: avoid corruption when deleting misaligned blocks
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6715/3/src/kudu/fs/log_block_manager-test-util.cc
File src/kudu/fs/log_block_manager-test-util.cc:

PS3, Line 295: uint64_t block_length = rand_.Uniform(fs_block_size * 4);
 :   block_length -= block_length % block_id_str_len;
 :   uint8_t data[block_length];
 :   for (int i = 0; i < ARRAYSIZE(data); i += block_id_str_len) {
 : memcpy([i], block_id_str.data(), block_id_str_len);
 :   }
it's ok if this writes a 0 sized block, right?


http://gerrit.cloudera.org:8080/#/c/6715/3/src/kudu/fs/log_block_manager-test.cc
File src/kudu/fs/log_block_manager-test.cc:

PS3, Line 782: Need to reopen the block manager after each corruption.
why?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ed6e349ab10d8d04722cd7ef99e7a06554f51f1
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] log block manager: more container accounting

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: log block manager: more container accounting
..


Patch Set 3: Code-Review+2

(1 comment)

one comment on some pre-existing code, but doesn't need to block this patch

http://gerrit.cloudera.org:8080/#/c/6716/3/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

Line 802: LOG(WARNING) << Substitute(
i'm wondering why this isn't FATAL


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1907db2a74bcc074ecf882f8a758c3989431e267
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [KUDU-754] add an environment variable for kudu client debugging to stderr

[William Li (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

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

to look at the new patch set (#2).

Change subject: [KUDU-754] add an environment variable for kudu client 
debugging to stderr
..

[KUDU-754] add an environment variable for kudu client debugging to stderr

Read environment variable "KUDU_CLIENT_VERBOSE" to get verbose level.
Calls SetVerboseLogLevel to set the specific verbose level.
Notes: The InitiGoogleLoggingSafeBasic() already sets the debugging to stderr.

Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
---
M src/kudu/client/client-internal.h
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
3 files changed, 33 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/36/6736/2
-- 
To view, visit http://gerrit.cloudera.org:8080/6736
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iab5c7c24395c25184489200283dd38da024c07bb
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1978: avoid corruption when deleting misaligned blocks

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1978: avoid corruption when deleting misaligned blocks
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6715/3/src/kudu/fs/log_block_manager-test-util.cc
File src/kudu/fs/log_block_manager-test-util.cc:

PS3, Line 288: id
maybe say stringified id? at first i expected to just have the blockid in 
binary format (int64s). In fact would that be simpler? we always know it'll be 
int64


http://gerrit.cloudera.org:8080/#/c/6715/3/src/kudu/fs/log_block_manager-test-util.h
File src/kudu/fs/log_block_manager-test-util.h:

PS3, Line 66: sequences of its size
of its block id?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ed6e349ab10d8d04722cd7ef99e7a06554f51f1
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] log block manager: corruptor test utility

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: log block manager: corruptor test utility
..


Patch Set 11: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I197b693d5a3c1a909e91b772ebfb31e50bae488b
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] fs: generate report during Open

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: fs: generate report during Open
..


Patch Set 10: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idcc1d512e2bed103c8d41623a63b4d071532b35e
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] fs: generate report during Open

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: fs: generate report during Open
..


Patch Set 10: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idcc1d512e2bed103c8d41623a63b4d071532b35e
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] WIP: Add tool to detect pending config changes

[Mike Percy (Code Review)]

Mike Percy has uploaded a new change for review.

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

Change subject: WIP: Add tool to detect pending config changes
..

WIP: Add tool to detect pending config changes

This is a first pass at a tool that prints the active config on all
replicas the master knows about for a specified tablet id.

Change-Id: I38dee145ecd15119c1d6f732d9f79ce8bcf86f70
---
M src/kudu/tools/tool_action_tablet.cc
1 file changed, 85 insertions(+), 7 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I38dee145ecd15119c1d6f732d9f79ce8bcf86f70
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 

[kudu-CR] WIP: KUDU-463. Add checksumming to cfile

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: WIP: KUDU-463. Add checksumming to cfile
..


Patch Set 11:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6630/11/src/kudu/util/env.h
File src/kudu/util/env.h:

Line 375:   Slice*   result;  // Set to the data and length that was read (OUT)
Since there are no short reads, can we omit 'result' altogether? Without short 
reads, ReadV() either fails (in which case 'entries' is totally invalid) or it 
succeeds fully (in which case the results of the read is described fully by 
each 'length' and 'scratch' pair). If the caller needs slices, it can make its 
own.

Alternatively, perhaps we can replace vector with vector? 
Each slice describes both 'length' and 'scratch', and would be used as input 
when constructing the iov internally.

In either case, ReadV() won't modify the vector and so it should be passed by 
const ref to avoid needless copies. An alternative would to be pass it by value 
and move it, but I think the caller still needs access after ReadV(), and it 
would lose access if move were used.

Given the amount of discussion around this interface, I really think this part 
of the patch should be split out into its own gerrit change, partly to ease 
code review and partly so the new git commit can more fully document the design 
trade-offs taken.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6756834cd7f27af258797a3654a95244abeb0976
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] subprocess: remove progname argument

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: subprocess: remove progname argument
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I42d3cb551c350d8f10308035084a8807a1ae240b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1982 Java client calls NetworkInterface.getByInetAddress too often NetworkInterface.getByInetAddress (which is used by NetUtil.isLocalAddress) is very slow on Windows, so cache added.

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: KUDU-1982 Java client calls NetworkInterface.getByInetAddress 
too often NetworkInterface.getByInetAddress (which is used by 
NetUtil.isLocalAddress) is very slow on Windows, so cache added.
..


KUDU-1982 Java client calls NetworkInterface.getByInetAddress too often
NetworkInterface.getByInetAddress (which is used by NetUtil.isLocalAddress) is 
very slow on Windows, so cache added.

Change-Id: I8fc9d40ebffa17ef43973dc23237dd31dd309c06
Reviewed-on: http://gerrit.cloudera.org:8080/6735
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java
A java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java
2 files changed, 44 insertions(+), 1 deletion(-)

Approvals:
  Todd Lipcon: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I8fc9d40ebffa17ef43973dc23237dd31dd309c06
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Pavel Martynov 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Pavel Martynov 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1982 Java client calls NetworkInterface.getByInetAddress too often NetworkInterface.getByInetAddress (which is used by NetUtil.isLocalAddress) is very slow on Windows, so cache added.

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1982 Java client calls NetworkInterface.getByInetAddress 
too often NetworkInterface.getByInetAddress (which is used by 
NetUtil.isLocalAddress) is very slow on Windows, so cache added.
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8fc9d40ebffa17ef43973dc23237dd31dd309c06
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Pavel Martynov 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Pavel Martynov 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] WIP: KUDU-463. Add checksumming to cfile

[Grant Henke (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

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

to look at the new patch set (#11).

Change subject: WIP: KUDU-463. Add checksumming to cfile
..

WIP: KUDU-463. Add checksumming to cfile

Adds optional checksumming and validation to cfile blocks.

Introduces 2 flags to control cfile checksumming:
- cfile_write_checksums (default false)
- cfile_verify_checksums (default true)

cfile_write_checksums is used in the CFileWriter to enable computing and
appending Crc32 checksums to the end of each cfile block, header and footer

cfile_write_checksums is defaulted to false to ensure upgrades don't
immediately result in performance degredation and incompatible data on
downgrade. It can and likely should be defaulted to true in a later release.

When cfile_write_checksums is set to true, the existing incompatible_features
bitset in the cfile footer is used. A "checksum" bit is set to ensure a clear
error message when older versions of Kudu try to read the file. If checksums
are not written the incompatible_features "checksum" bit is not set.

cfile_verify_checksums is used in the CFileReader to enable validating the
data against the written checksum. cfile_verify_checksums is defaulted to
true since validation only occurs if checksums are written. Any data that
was written before checksumming was an option or when cfile_write_checksums
was false will not be verified.

Change-Id: I6756834cd7f27af258797a3654a95244abeb0976
---
M docs/design-docs/cfile.md
M src/kudu/cfile/cfile-test.cc
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/cfile_reader.h
M src/kudu/cfile/cfile_util.h
M src/kudu/cfile/cfile_writer.cc
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/block_manager.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/fs-test-util.h
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/crc-test.cc
M src/kudu/util/crc.cc
M src/kudu/util/crc.h
M src/kudu/util/env-test.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/file_cache.cc
18 files changed, 724 insertions(+), 63 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/6630/11
-- 
To view, visit http://gerrit.cloudera.org:8080/6630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6756834cd7f27af258797a3654a95244abeb0976
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] WIP: KUDU-463. Add checksumming to cfile

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: WIP: KUDU-463. Add checksumming to cfile
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6630/10/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:

Line 174: // Simulates Linux's preadv API on OS X.
> DoWriteV actually changes it entire implementation instead of simulating pw
Sure, works for me.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6756834cd7f27af258797a3654a95244abeb0976
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] subprocess: add ScopedSubprocess

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: subprocess: add ScopedSubprocess
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6741/1/src/kudu/integration-tests/full_stack-insert-scan-test.cc
File src/kudu/integration-tests/full_stack-insert-scan-test.cc:

PS1, Line 232:   string cmd = Substitute("perf record --pid=$0", getpid());
 :   if (FLAGS_perf_fp_flag) cmd += " --call-graph fp";
> I think the original code here intended that you should pass --perf_fp_flag
What systems are those? On the two that I tested (Ubuntu 16 and el6.6) "perf 
record --call-graph" complained that --call-graph requires a value (i.e. " fp").


http://gerrit.cloudera.org:8080/#/c/6741/1/src/kudu/util/subprocess.h
File src/kudu/util/subprocess.h:

Line 175: // Subprocess that is automatically killed when it goes out of scope.
> I'm not entirely following this, cuz Subprocess itself already does automat
MakeScopedCleanup() doesn't work so well if you want to embed the cleanup 
itself in another object (the type is tricky to deduce which is why we use 
'auto' everywhere). You can see an example of that in my next patch.

But, you're right that this does reproduce much of ~Subprocess(). I'll change 
this to 1) a way to customize the signal sent in ~Subprocess, and 2) a fallback 
to SIGKILL if the custom signal takes too long to deliver.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf31bd4ea6de2917521a9852714fb33cfbec1f61
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-579 [java client] Scanner fault tolerance

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change.

Change subject: KUDU-579 [java_client] Scanner fault tolerance
..


Patch Set 9:

The failure test passed on my local dev environment. And it seems not related 
to the change?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I89d3634c4255b69e28f2de5412e6a5a9d34e931b
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] WIP: KUDU-463. Add checksumming to cfile

[Grant Henke (Code Review)]

Grant Henke has posted comments on this change.

Change subject: WIP: KUDU-463. Add checksumming to cfile
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6630/10/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:

Line 174: // Simulates Linux's preadv API on OS X.
> BTW, I just noticed that we simulate pwritev() inline, in DoWriteV(). If yo
DoWriteV actually changes it entire implementation instead of simulating 
pwritev. I can definitely move this, but it might be best in a follow up patch.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6756834cd7f27af258797a3654a95244abeb0976
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] WIP: KUDU-463. Add checksumming to cfile

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: WIP: KUDU-463. Add checksumming to cfile
..


Patch Set 10:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6630/10/src/kudu/fs/block_manager-test.cc
File src/kudu/fs/block_manager-test.cc:

Line 262:   gscoped_ptr scratch1(new uint8_t[size1]);
> I mainly just followed the "pattern" from a few lines above. Happy to chang
Yeah, we should be using unique_ptr in new sites. Not sure why I used heap 
allocation above, but that can change too.


http://gerrit.cloudera.org:8080/#/c/6630/10/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:

Line 174: // Simulates Linux's preadv API on OS X.
BTW, I just noticed that we simulate pwritev() inline, in DoWriteV(). If you're 
feeling generous perhaps you can pull that out into this area too, so the two 
simulated functions can be defined side by side?


Line 334:   virtual Status ReadV(uint64_t offset, vector results) 
const OVERRIDE {
> It doesn't look like any read or write calls have this now. Since these are
Hmm you're probably right; I didn't realize there weren't any calls in Write 
either. Nevermind then.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6756834cd7f27af258797a3654a95244abeb0976
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [docs] Add troubleshooting for KuduStorageHandler

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has submitted this change and it was merged.

Change subject: [docs] Add troubleshooting for KuduStorageHandler
..


[docs] Add troubleshooting for KuduStorageHandler

Change-Id: I80e028a6f827269d97f26ec7a2cf4b8c22d2a838
Reviewed-on: http://gerrit.cloudera.org:8080/6738
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert 
---
M docs/troubleshooting.adoc
1 file changed, 10 insertions(+), 0 deletions(-)

Approvals:
  Dan Burkert: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I80e028a6f827269d97f26ec7a2cf4b8c22d2a838
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [docs] Add troubleshooting for KuduStorageHandler

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: [docs] Add troubleshooting for KuduStorageHandler
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I80e028a6f827269d97f26ec7a2cf4b8c22d2a838
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] WIP: KUDU-463. Add checksumming to cfile

[Grant Henke (Code Review)]

Grant Henke has posted comments on this change.

Change subject: WIP: KUDU-463. Add checksumming to cfile
..


Patch Set 10:

(23 comments)

http://gerrit.cloudera.org:8080/#/c/6630/10/src/kudu/cfile/cfile_reader.cc
File src/kudu/cfile/cfile_reader.cc:

Line 301:   RETURN_NOT_OK(block_->ReadV(off, results));
> I think it would be a little less obtrusive to have ReadResult construct an
My goal was to match the pattern and variables used in the Read function. 
Especially since this is essentially a version of Read for multiple Slices.


http://gerrit.cloudera.org:8080/#/c/6630/10/src/kudu/cfile/cfile_reader.h
File src/kudu/cfile/cfile_reader.h:

Line 210:   bool has_checksums_;
> This will be padded to 8 bytes, which is actually quite a bit given how man
Done


http://gerrit.cloudera.org:8080/#/c/6630/10/src/kudu/fs/block_manager-test.cc
File src/kudu/fs/block_manager-test.cc:

Line 262:   gscoped_ptr scratch1(new uint8_t[size1]);
> Nit: use unique_ptr.
I mainly just followed the "pattern" from a few lines above. Happy to change it.


Line 267:   ReadResult {
> Surprised that this style of struct initialization works; isn't it C99 but 
Will change this.


http://gerrit.cloudera.org:8080/#/c/6630/10/src/kudu/fs/block_manager.h
File src/kudu/fs/block_manager.h:

Line 162:   // Does not modify 'result' on error (but may modify 'scratch').
> You mean results?
I mean the result field in the results. I will make this more clear.


Line 163:   virtual Status ReadV(uint64_t offset, vector results) 
const = 0;
> nit: Consider making this vector* perhaps, based on my comments
Looking into this. Adar mentioned it too.


http://gerrit.cloudera.org:8080/#/c/6630/10/src/kudu/fs/fs-test-util.h
File src/kudu/fs/fs-test-util.h:

Line 72:   virtual Status ReadV(uint64_t offset, vector results) 
const OVERRIDE {
> This is a header so we retain namespace qualifications (i.e. this should be
Done


http://gerrit.cloudera.org:8080/#/c/6630/10/src/kudu/util/crc.cc
File src/kudu/util/crc.cc:

Line 50:   uint64_t crc_tmp =  static_cast(crc32);
> Nit: extra space here.
Done


http://gerrit.cloudera.org:8080/#/c/6630/10/src/kudu/util/crc.h
File src/kudu/util/crc.h:

Line 38: uint32_t Crc32c(const void* data, size_t length, uint32_t crc32);
> Maybe previous_crc32 to be more clear?
Done


Line 38: uint32_t Crc32c(const void* data, size_t length, uint32_t crc32);
> Maybe name it RollingCrc32()? Also, needs a unit test.
I though it was nice to have the same name as the original function since it 
works in tandem with it. 
ie:
uint32_t crc = Crc32c(data, length);
crc = Crc32c(data, length, crc);


http://gerrit.cloudera.org:8080/#/c/6630/10/src/kudu/util/env.h
File src/kudu/util/env.h:

Line 371: struct ReadResult {
> Should provide a short high-level struct document too.
Done


Line 403:   virtual Status ReadV(uint64_t offset, std::vector 
results) const = 0;
> I'm finding this to be somewhat confusing.
I agree. I had a hard time naming the struct too. I am not sure ReadEntry is 
perfect either, but it's definitely better. 

Will change and re-organize ReadResult.


Line 536:   // TODO (GH): Document
> Yep. :)
I was waiting until it was decided if I should allow short reads here or not. I 
will move the readfully logic here and document accordingly.


http://gerrit.cloudera.org:8080/#/c/6630/10/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:

Line 175: ssize_t preadv(int fd, const struct iovec *iovec, int count, off_t 
offset) {
> Should be const struct iovec*
Done


Line 180:   return read_bytes;
> Nit: if you're going to elide the braces, merge this line into the conditio
Done


Line 182:   break;
> Don't we need to update total_read_bytes with our partial read results befo
Done


Line 334:   virtual Status ReadV(uint64_t offset, vector results) 
const OVERRIDE {
> Could you add TRACE_EVENT1() calls to ReadV() and Read()? Not sure why they
It doesn't look like any read or write calls have this now. Since these are 
called frequently is there some overhead to worry about?


Line 341:   return iovec {
> Again, surprised this kind of initialization works in C++11.
Done


Line 353: // Adjust slice sizes based on bytes read for short reads
> As we discussed, let's make short reads an error, for Read() too. Do it as 
Will do.


Line 355: for (ReadResult& readResult : results) {
> Nit: auto is fine for short-scoped types like this one.
Done


http://gerrit.cloudera.org:8080/#/c/6630/10/src/kudu/util/env_util.cc
File src/kudu/util/env_util.cc:

Line 196:   size_t n = accumulate(results.begin(), results.end(), 
static_cast(0),
> nit: name this req_len ?
I can rename this. Often when in a new codebase I try to match the style that 
exists. Given this is a "mirror" of ReadFully above I used similar names.


Line 207: // Calculate the read amount of data
> Nit: Use punctuation like periods at the end of comment sentences per https

[kudu-CR] Allow to pad UNIXTIME MICROS slots in scan results

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has submitted this change and it was merged.

Change subject: Allow to pad UNIXTIME_MICROS slots in scan results
..


Allow to pad UNIXTIME_MICROS slots in scan results

This changes the wire protocol to, upon request, pad
the slots that contain values for UNIXTIME_MICROS columns
by 8 bytes. This allows Impala to adopt the result of a
Kudu scan as a whole while still having space to transform
UNIXTIME_MICROS values, which occupy 8 bytes, to the Impala
representation of timestamp, which occupies 16, in place
and without copying memory.

This patch doesn't include any de-serialization logic
the reasoning being that Impala will have knowledge of
the data format in order to perform de-serialization
directly on the "raw" direct and indirect data.

This patch doesn't introduce any branching in the
serialization patch. It does move the memset() call
that was performed once per nullable column, per row,
to be performed on the whole block instead. While less
cache friendly, this is also executed less times. The
net gain is not significant, but it does not regress
in the normal case.

Results of running the wire_protocol-test benchmark
in slow mode:

Before (avg 3 runs): 3.076
After  (avg 3 runs): 3.000

The difference is around -2%, which might be in the noise
but demostrates no significant regression.

Change-Id: I99fc6d3be089d19ebe2e70c938f2405c381578b4
Reviewed-on: http://gerrit.cloudera.org:8080/6623
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/common/wire_protocol-test.cc
M src/kudu/common/wire_protocol.cc
M src/kudu/common/wire_protocol.h
3 files changed, 200 insertions(+), 48 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I99fc6d3be089d19ebe2e70c938f2405c381578b4
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] subprocess: add ScopedSubprocess

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: subprocess: add ScopedSubprocess
..


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/6741/1/src/kudu/integration-tests/full_stack-insert-scan-test.cc
File src/kudu/integration-tests/full_stack-insert-scan-test.cc:

PS1, Line 232:   string cmd = Substitute("perf record --pid=$0", getpid());
 :   if (FLAGS_perf_fp_flag) cmd += " --call-graph fp";
I think the original code here intended that you should pass --perf_fp_flag on 
the systems that require the 'fp' parameter, but that itwould _always_ capture 
the call graph.


http://gerrit.cloudera.org:8080/#/c/6741/1/src/kudu/util/subprocess.h
File src/kudu/util/subprocess.h:

Line 175: // Subprocess that is automatically killed when it goes out of scope.
I'm not entirely following this, cuz Subprocess itself already does 
automatically kill itself when going out of scope, doesn't it? The difference 
here only seems to be that the signal can be set, and it waits for the exit 
code, rather than the default which does -9?

One thought: what if you move the 'kill and wait' into a member function of 
subprocess itself, and then it would be reasonably easy to just use a 
MakeScopedCleanup([&]() { p.KillAndWait(); } or whatever instead of this RAII 
helper?


Line 185:   ~ScopedSubprocess() {
nit: given this is a pretty heavy-weight function, i think it's better to 
define out of line in the .cc


Line 190: WARN_NOT_OK(p_.Kill(signum_), "Could not kill subprocess");
would be useful to include argv[0] or something in these warning messages here 
and below


Line 191: Status s = p_.Wait();
should there be any kind of timeout and fallback to kill -9?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaf31bd4ea6de2917521a9852714fb33cfbec1f61
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1982 Java client calls NetworkInterface.getByInetAddress too often. Remove forgotten debug code.

[Pavel Martynov (Code Review)]

Pavel Martynov has abandoned this change.

Change subject: KUDU-1982 Java client calls NetworkInterface.getByInetAddress 
too often. Remove forgotten debug code.
..


Abandoned

squashed

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ic325db600b0f9ce698f2f3e08c1af1f0a0e411d6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Pavel Martynov 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1982 Java client calls NetworkInterface.getByInetAddress too often NetworkInterface.getByInetAddress (which is used by NetUtil.isLocalAddress) is very slow on Windows, so cache added.

[Pavel Martynov (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

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

to look at the new patch set (#3).

Change subject: KUDU-1982 Java client calls NetworkInterface.getByInetAddress 
too often NetworkInterface.getByInetAddress (which is used by 
NetUtil.isLocalAddress) is very slow on Windows, so cache added.
..

KUDU-1982 Java client calls NetworkInterface.getByInetAddress too often
NetworkInterface.getByInetAddress (which is used by NetUtil.isLocalAddress) is 
very slow on Windows, so cache added.

Change-Id: I8fc9d40ebffa17ef43973dc23237dd31dd309c06
---
M java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java
A java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java
2 files changed, 44 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/6735/3
-- 
To view, visit http://gerrit.cloudera.org:8080/6735
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8fc9d40ebffa17ef43973dc23237dd31dd309c06
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Pavel Martynov 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Pavel Martynov 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1981 Kudu should run at hosts len(FQDN) > 64

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1981 Kudu should run at hosts len(FQDN) > 64
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6734/2/src/kudu/security/ca/cert_management-test.cc
File src/kudu/security/ca/cert_management-test.cc:

PS2, Line 179: // Verify that it's possible to set the common name field up to 
the maximum of
 : // 64 characters while generating request and signing the 
certificate. The CN
 : // (common name) field goes into the subject name of the result 
certificate
 : // along with the UID (user id) field.
 : T
not following the purpose of this test. it seems like we're redundantly testing 
OpenSSL here


http://gerrit.cloudera.org:8080/#/c/6734/2/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:

Line 297:   config->hostnames = { fqdn };
if we're now only using the SAN field, can we just get rid of the CN field 
entirely, rather than keeping the dead code path? (maybe I'm misunderstanding). 
Seems like we could kill off a number of lines of code.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie142e76e9b2dcef3e07dd33d82b6758c746ced19
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [c++-client] clear non-covered entries from meta cache on table open

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: [c++-client] clear non-covered entries from meta cache on table 
open
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I195eac8caa5efca9ad95284ab707c38340ba47f0
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1982 Java client calls NetworkInterface.getByInetAddress too often. Remove forgotten debug code.

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1982 Java client calls NetworkInterface.getByInetAddress 
too often. Remove forgotten debug code.
..


Patch Set 1:

Hi Pavel. it looks like you've ended up with three commits now for this. Gerrit 
doesn't do well with "patch series". Can you squash all three down to a single 
commit and Abandon the remaining two?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic325db600b0f9ce698f2f3e08c1af1f0a0e411d6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Pavel Martynov 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] WIP: KUDU-463. Add checksumming to cfile

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: WIP: KUDU-463. Add checksumming to cfile
..


Patch Set 10:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/6630/10/src/kudu/cfile/cfile_reader.cc
File src/kudu/cfile/cfile_reader.cc:

PS10, Line 277:   uint8_t footer_scratch[footer_size];
  :   Slice footer;
  : 
  :   uint32_t checksum_size = sizeof(uint32_t);
  :   uint8_t checksum_scratch[checksum_size];
  :   Slice checksum;
  : 
  :   // Read both the header and checksum in one call.
  :   // We read the checksum position in case one exists.
  :   // This is done to avoid the need for a follow up read call.
  :   // Read both the data and checksum in one call
  :   vector results = {
  :   ReadResult {
  :   .result = ,
  :   .scratch = checksum_scratch,
  :   .length = checksum_size,
  :   },
  :   ReadResult {
  :   .result = ,
  :   .scratch = footer_scratch,
  :   .length = footer_size,
  :   }
  :   };
  :   uint64_t off = file_size_ - kMagicAndLengthSize - footer_size 
- checksum_size;
  :   RETURN_NOT_OK(block_->ReadV(off, results));
I think it would be a little less obtrusive to have ReadResult construct and 
own the Slice, the scratch buffer (you could use faststring) and the length and 
use a constructor like

  explicit ReadResult(buffer_size)

So then instead of all this you could just say

  uint32_t checksum_size = sizeof(uint32_t);
  vector results = { ReadResult(checksum_size), 
ReadResult(footer_size) };
  uint64_t off = file_size_ - kMagicAndLengthSize - footer_size - checksum_size;
  RETURN_NOT_OK(block_->ReadV(off, ));

And avoid copying the vector.


http://gerrit.cloudera.org:8080/#/c/6630/10/src/kudu/fs/block_manager.h
File src/kudu/fs/block_manager.h:

PS10, Line 163: vector
nit: Consider making this vector* perhaps, based on my comments 
elsewhere


http://gerrit.cloudera.org:8080/#/c/6630/10/src/kudu/util/crc.h
File src/kudu/util/crc.h:

PS10, Line 38: Crc32c
Maybe name it RollingCrc32()? Also, needs a unit test.


http://gerrit.cloudera.org:8080/#/c/6630/10/src/kudu/util/env_util.cc
File src/kudu/util/env_util.cc:

PS10, Line 196: n
nit: name this req_len ?


Line 207: // Calculate the read amount of data
Nit: Use punctuation like periods at the end of comment sentences per 
https://google.github.io/styleguide/cppguide.html#Punctuation,_Spelling_and_Grammar


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6756834cd7f27af258797a3654a95244abeb0976
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Allow to pad UNIXTIME MICROS slots in scan results

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Allow to pad UNIXTIME_MICROS slots in scan results
..


Patch Set 10: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I99fc6d3be089d19ebe2e70c938f2405c381578b4
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [rpc] handling ERROR UNAVAILABLE RPC error

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [rpc] handling ERROR_UNAVAILABLE RPC error
..


Patch Set 10:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/6640/10/src/kudu/integration-tests/security-unknown-tsk-itest.cc
File src/kudu/integration-tests/security-unknown-tsk-itest.cc:

Line 164:   void ReplaceAuthnToken(shared_ptr client,
> warning: the parameter 'client' is copied for each invocation but only used
Done


Line 164:   void ReplaceAuthnToken(shared_ptr client,
> Pass a KuduClient*.
Good idea -- for some reason I didn't come up with that simple solution.


PS10, Line 320:  cutsom
> typo
Done


Line 328:   auto importer_cleanup = MakeScopedCleanup([&]() {
> Is this join necessary?
It's necessary if we are about to gracefully complete the test if something 
goes wrong in the code below.

Otherwise it would be a mess during the destruction of the tread if the thread 
were not joined by the time of destruction.  I saw several cases like that in 
other tests.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [rpc] handling ERROR UNAVAILABLE RPC error

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

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

to look at the new patch set (#11).

Change subject: [rpc] handling ERROR_UNAVAILABLE RPC error
..

[rpc] handling ERROR_UNAVAILABLE RPC error

This patch adds handling of the newly introduced ERROR_UNAVAILABLE RPC
error code.  The ERROR_UNAVAILABLE error code is a broader counterpart
of the ERROR_SERVER_TOO_BUSY.

>From the client side, both ERROR_UNAVAILABLE and ERROR_SERVER_TOO_BUSY
codes mean it's worth retrying the call at a later time.  To reflect
that, the internal codes {RetriableRpcStatus,ScanRpcStatus}::SERVER_BUSY
are replaced with more generic SERVICE_UNAVAILABLE.

Added an integration test to cover the behavior of the server components
and the Kudu C++ client when client sends authn token signed by a TSK
unknown to master and tablet servers.

Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959
---
M src/kudu/client/batcher.cc
M src/kudu/client/client-internal.cc
M src/kudu/client/client.h
M src/kudu/client/scanner-internal.cc
M src/kudu/client/scanner-internal.h
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/security-unknown-tsk-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/integration-tests/test_workload.h
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/exactly_once_rpc-test.cc
M src/kudu/rpc/outbound_call.h
M src/kudu/rpc/retriable_rpc.h
M src/kudu/rpc/rpc.cc
M src/kudu/rpc/rpc.h
M src/kudu/rpc/rpc_context.h
16 files changed, 533 insertions(+), 35 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/6640/11
-- 
To view, visit http://gerrit.cloudera.org:8080/6640
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] mapreduce: add support for fault tolerant scanner

[Hao Hao (Code Review)]

Hao Hao has uploaded a new change for review.

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

Change subject: mapreduce: add support for fault tolerant scanner
..

mapreduce: add support for fault tolerant scanner

This adds support to use fault tolerant scanner in mapreduce job.
By default non fault tolerant scanner is used. To turn on fault
tolerant scanner, use job config:
'kudu.mapreduce.input.fault.tolerant.scan'.

Change-Id: Ibc39472e2733bab4e00e73658f8a7619153bd7c6
---
M 
java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableInputFormat.java
M 
java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/KuduTableMapReduceUtil.java
M 
java/kudu-mapreduce/src/test/java/org/apache/kudu/mapreduce/ITInputFormatJob.java
3 files changed, 23 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibc39472e2733bab4e00e73658f8a7619153bd7c6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 

[kudu-CR] KUDU-579 [java client] Scanner fault tolerance

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change.

Change subject: KUDU-579 [java_client] Scanner fault tolerance
..


Patch Set 9:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/6566/8/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:

PS8, Line 731: Table
> nit: doesn't need to be final.
Done


http://gerrit.cloudera.org:8080/#/c/6566/8/java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java:

PS8, Line 124: 
> nit, remove
Done


http://gerrit.cloudera.org:8080/#/c/6566/8/java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java:

Line 515: if (errCode == Tserver.TabletServerErrorPB.Code.TABLET_NOT_FOUND) 
{
> master.proto says:
Done


http://gerrit.cloudera.org:8080/#/c/6566/8/java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
File 
java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java:

Line 132:   killTabletLeader(scanner.currentTablet());
> Can you add tests that do the same as below but that aren't fault tolerant?
Done


Line 159
> Can you also add tests that kill/restart servers after we're done scanning 
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I89d3634c4255b69e28f2de5412e6a5a9d34e931b
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] KUDU-579 [java client] Scanner fault tolerance

[Hao Hao (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

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

to look at the new patch set (#9).

Change subject: KUDU-579 [java_client] Scanner fault tolerance
..

KUDU-579 [java_client] Scanner fault tolerance

This patch adds java client support to restart scanners if they
fail in the middle of table scanning. AsyncKuduScanner records
the final primary key retrieved in the previous batch. If a tserver
fails while scanning, the client marks the tserver as failed and
retries the scan elsewhere, providing its last primary key.

Change-Id: I89d3634c4255b69e28f2de5412e6a5a9d34e931b
---
M 
java/kudu-client/src/main/java/org/apache/kudu/client/AbstractKuduScannerBuilder.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanToken.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RowResultIterator.java
A 
java/kudu-client/src/main/java/org/apache/kudu/client/ScannerExpiredException.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
A 
java/kudu-client/src/test/java/org/apache/kudu/client/ITFaultTolerantScanner.java
A 
java/kudu-client/src/test/java/org/apache/kudu/client/ITNonFaultTolerantScanner.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/ITScannerMultiTablet.java
12 files changed, 467 insertions(+), 98 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/6566/9
-- 
To view, visit http://gerrit.cloudera.org:8080/6566
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I89d3634c4255b69e28f2de5412e6a5a9d34e931b
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-1982 Java client calls NetworkInterface.getByInetAddress too often. Remove forgotten debug code.

[Pavel Martynov (Code Review)]

Pavel Martynov has uploaded a new change for review.

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

Change subject: KUDU-1982 Java client calls NetworkInterface.getByInetAddress 
too often. Remove forgotten debug code.
..

KUDU-1982 Java client calls NetworkInterface.getByInetAddress too often.
Remove forgotten debug code.

Change-Id: Ic325db600b0f9ce698f2f3e08c1af1f0a0e411d6
---
M java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java
1 file changed, 2 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic325db600b0f9ce698f2f3e08c1af1f0a0e411d6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Pavel Martynov