[kudu-CR] KUDU-2426 Fix WRONG SERVER UUID case in ksck

[Andrew Wong (Code Review)]

Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10293 )

Change subject: KUDU-2426 Fix WRONG_SERVER_UUID case in ksck
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10293/4/src/kudu/tools/ksck-test.cc
File src/kudu/tools/ksck-test.cc:

http://gerrit.cloudera.org:8080/#/c/10293/4/src/kudu/tools/ksck-test.cc@477
PS4, Line 477: TEST_F(KsckTest, TestWrongUUIDTabletServer) {
> Hmm.. maybe also add a regression test specific to KUDU-2426 that other rem
My mistake, I overlooked ksck_remote-test.cc! Feel free to ignore my suggestion.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b4f50fe4dd94450b4f2e34dbad315bd761b071f
Gerrit-Change-Number: 10293
Gerrit-PatchSet: 4
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 11 May 2018 02:49:20 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191: Metadata Upgrade Tool

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10075 )

Change subject: KUDU-2191: Metadata Upgrade Tool
..


Patch Set 13:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/10075/12/src/kudu/hms/hms_catalog-test.cc
File src/kudu/hms/hms_catalog-test.cc:

http://gerrit.cloudera.org:8080/#/c/10075/12/src/kudu/hms/hms_catalog-test.cc@237
PS12, Line 237: table.__set_parameters({
> I think it would be better to remove this behavior from the KuduMetastorePl
Done


http://gerrit.cloudera.org:8080/#/c/10075/12/src/kudu/hms/hms_catalog-test.cc@456
PS12, Line 456: hms_tables_map, kExternalTableName));
> instead of calling GetTable and passing in the parameter, perhaps it's bett
Done


http://gerrit.cloudera.org:8080/#/c/10075/12/src/kudu/hms/hms_catalog.h
File src/kudu/hms/hms_catalog.h:

http://gerrit.cloudera.org:8080/#/c/10075/12/src/kudu/hms/hms_catalog.h@91
PS12, Line 91:   const std::string& db_name,
> This is the name of the table as stored in the HMS, right?  If so, it might
Done


http://gerrit.cloudera.org:8080/#/c/10075/12/src/kudu/hms/hms_catalog.h@96
PS12, Line 96: or a
> nit: 'is the legacy...'
Done


http://gerrit.cloudera.org:8080/#/c/10075/12/src/kudu/hms/hms_catalog.h@100
PS12, Line 100:   // This method will fail if the HMS is unreachable.
> I'm not quite following this bit - why is storage_handler taken as an argum
In the following patch, the same method is used to retrieve non-legacy tables. 
So it takes storage_handler as an argument. The reason why the need to have a 
map is to avoid O(n) table lookup given a Kudu table's name.


http://gerrit.cloudera.org:8080/#/c/10075/12/src/kudu/hms/hms_catalog.cc
File src/kudu/hms/hms_catalog.cc:

http://gerrit.cloudera.org:8080/#/c/10075/12/src/kudu/hms/hms_catalog.cc@193
PS12, Line 193:
> nit: 'non-legacy'
Done


http://gerrit.cloudera.org:8080/#/c/10075/12/src/kudu/tools/tool.proto
File src/kudu/tools/tool.proto:

http://gerrit.cloudera.org:8080/#/c/10075/12/src/kudu/tools/tool.proto@43
PS12, Line 43:   // Whether or not to create a Hive Metastore, and/or enable 
Kudu Hive
 :   // Metastore integration.
 :   optional HmsMode hms_mode = 7;
 :
 :   // The directory where the cluster's data and l
> Same feedback.
Done


http://gerrit.cloudera.org:8080/#/c/10075/12/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:

http://gerrit.cloudera.org:8080/#/c/10075/12/src/kudu/tools/tool_action_hms.cc@67
PS12, Line 67: non-Impala
> 'non-Impala' here and below (non should usually be hyphen separated from th
Done


http://gerrit.cloudera.org:8080/#/c/10075/12/src/kudu/tools/tool_action_hms.cc@136
PS12, Line 136: 
> This should probably go before creating the HmsCatalog.
Done


http://gerrit.cloudera.org:8080/#/c/10075/12/src/kudu/tools/tool_action_hms.cc@145
PS12, Line 145:   .master_server_addrs(master_addresses)
> Maybe add a comment here?  I'm not sure why this would need to have non-def
Oops, thanks for pointing it out. I think actually it is not needed.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1c8488e1c7b04b8bedc31c83b1496b53bae49cb3
Gerrit-Change-Number: 10075
Gerrit-PatchSet: 13
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 11 May 2018 01:50:06 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2191: Metadata Upgrade Tool

[Hao Hao (Code Review)]

Hello Tidy Bot, Alexey Serbin, Dan Burkert, Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: KUDU-2191: Metadata Upgrade Tool
..

KUDU-2191: Metadata Upgrade Tool

This commit introduces an upgrade tool to allows backward compatibility.
It provides support to upgrade the metadata format of existing Impala
managed/external tables in HMS entries, as well as rename the existing
tables in Kudu to adapt to the new naming rules. It adds test case
using external mini cluster to ensure the upgrade tool works as
expected.

Change-Id: I1c8488e1c7b04b8bedc31c83b1496b53bae49cb3
---
M 
java/kudu-hive/src/main/java/org/apache/kudu/hive/metastore/KuduMetastorePlugin.java
M 
java/kudu-hive/src/test/java/org/apache/kudu/hive/metastore/TestKuduMetastorePlugin.java
M src/kudu/client/client-test-util.cc
M src/kudu/client/client-test-util.h
M src/kudu/client/schema.h
M src/kudu/common/common.proto
M src/kudu/hms/hms_catalog-test.cc
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_catalog.h
M src/kudu/hms/hms_client.cc
M src/kudu/hms/hms_client.h
M src/kudu/integration-tests/alter_table-randomized-test.cc
M src/kudu/integration-tests/master_failover-itest.cc
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/mini-cluster/external_mini_cluster-test.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action.h
A src/kudu/tools/tool_action_hms.cc
M src/kudu/tools/tool_action_test.cc
M src/kudu/tools/tool_main.cc
24 files changed, 665 insertions(+), 50 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/75/10075/13
--
To view, visit http://gerrit.cloudera.org:8080/10075
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1c8488e1c7b04b8bedc31c83b1496b53bae49cb3
Gerrit-Change-Number: 10075
Gerrit-PatchSet: 13
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot

[kudu-CR] KUDU-2035 Enable HTTP compression for all webserver's paths

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10332 )

Change subject: KUDU-2035 Enable HTTP compression for all webserver's paths
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/util/curl_util.cc
File src/kudu/util/curl_util.cc:

http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/util/curl_util.cc@88
PS3, Line 88: chunk
> What happens if this function returns earlier than calling curl_slist_free_
Ah, it seems MakeScopedCleanup would fit better in here.  Anyway, make sure 
there is no memory leak even if this method returns early.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7c6db24b0fd2cbcca8a554460d310bd39ee5c071
Gerrit-Change-Number: 10332
Gerrit-PatchSet: 3
Gerrit-Owner: Fengling Wang 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Fengling Wang 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Fri, 11 May 2018 00:13:41 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2035 Enable HTTP compression for all webserver's paths

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10332 )

Change subject: KUDU-2035 Enable HTTP compression for all webserver's paths
..


Patch Set 3:

(19 comments)

http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/integration-tests/linked_list-test-util.h
File src/kudu/integration-tests/linked_list-test-util.h:

http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/integration-tests/linked_list-test-util.h@380
PS3, Line 380: enable and unenable
drop


http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/integration-tests/linked_list-test-util.h@381
PS3, Line 381: if (compression_enabled) {
 :   s = curl.FetchURL(url, , {"Accept-Encoding: 
gzip"});
 : } else {
 :   s = curl.FetchURL(url, );
 : }
you could use the ternary '? :' operator for brevity here.


http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/server/webserver-test.cc
File src/kudu/server/webserver-test.cc:

http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/server/webserver-test.cc@22
PS3, Line 22: #include 
: #include 
: #include 
: #include 
: #include 
re-order to be in alphabetic order


http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/server/webserver-test.cc@154
PS3, Line 154: size_t WriteCallback(void* buffer, size_t size, size_t nmemb, 
void* user_ptr) {
 :   size_t real_size = size * nmemb;
 :   faststring* buf = reinterpret_cast(user_ptr);
 :   CHECK_NOTNULL(buf)->append(reinterpret_cast(buffer), real_size);
 :   return real_size;
 : }
Is it used anywhere at all?


http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/server/webserver-test.cc@168
PS3, Line 168:   zlib::Uncompress(Slice(buf_.ToString()), );
Check for return status, i.e. wrap it into ASSERT_OK()


http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/server/webserver-test.cc@180
PS3, Line 180: gzip"
To test that the server's code extracts the supported encoding properly, also 
add other encodings into the list, e.g. 'deflate', 'bzip2', 'xz'.

In addition to that, have an additional case that specifies list of encodings 
that is not supported by Kudu's internal Web server.  For example, make a 
request with 'Accept-Encoding: megaturbogzip' and make sure the server does not 
return the gzipped content, because the server does not support the 
'megaturbogzip' encoding.


http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/server/webserver-test.cc@181
PS3, Line 181: Content-Encoding:
As a general note, HTTP header tags (or header names) are case-insensitive and 
the number of spaces after the column is arbitrary.


http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/server/webserver-test.cc@184
PS3, Line 184: unenabled
disabled?


http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/server/webserver-test.cc@188
PS3, Line 188: Check
Check for the


http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/server/webserver-test.cc@189
PS3, Line 189: ASSERT_STR_CONTAINS(buf_.ToString(), "X-Frame-Options: DENY");
Not sure whether this needs to be covered here.  Maybe, just check for one of 
the mandatory headers for HTTP/1.1, like 'Host' ?


http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/server/webserver-test.cc@191
PS3, Line 191: Check
ditto


http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/server/webserver.cc
File src/kudu/server/webserver.cc:

http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/server/webserver.cc@484
PS3, Line 484: accept_encoding_str
What if one of the supported encodings in the request were 'mygzip'?


http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/server/webserver.cc@483
PS3, Line 483:   bool is_compressed = false;
 :   if (accept_encoding_str
What about:

const bool is_compressed = accept_encoding_str && strstr(...);
if (is_compressed) {
  ...
}

?


http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/util/curl_util.h
File src/kudu/util/curl_util.h:

http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/util/curl_util.h@43
PS3, Line 43:   // Any existing data in the buffer is replaced.
Add a note about additional headers in the optional parameter.


http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/util/curl_util.cc
File src/kudu/util/curl_util.cc:

http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/util/curl_util.cc@88
PS3, Line 88: curl_slist *
Style: surrent code conventions dictate that it should be

struct curl_slist* chunk = nullptr;


http://gerrit.cloudera.org:8080/#/c/10332/3/src/kudu/util/curl_util.cc@88
PS3, Line 88: chunk
What happens if this function returns earlier than calling 
curl_slist_free_all(chunk)?

Consider using unique_ptr with custom deleter to wrap curl_slist.  You can see 
examples at https://en.cppreference.com/w/cpp/memory/unique_ptr

[kudu-CR] [tools] ksck improvements [7/n] Add JSON output option to ksck

[Andrew Wong (Code Review)]

Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10288 )

Change subject: [tools] ksck improvements [7/n] Add JSON output option to ksck
..


Patch Set 10: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5da0752f8e41c022611253c300450368f6ae969
Gerrit-Change-Number: 10288
Gerrit-PatchSet: 10
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 10 May 2018 23:35:59 +
Gerrit-HasComments: No

[kudu-CR] Add a column renaming tool

[Hao Hao (Code Review)]

Hao Hao has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10368 )

Change subject: Add a column renaming tool
..

Add a column renaming tool

This commit introduces a tool to rename a table's column. Similar to
table renaming tool, when HMS integration feature is enabled, users can
use this tool to rename table's column that have hive incompatible
names. In order to allow these tables to be upgraded.

Change-Id: Ibe98ce52d6865fa61c0903b38cfac3e86fc05a66
Reviewed-on: http://gerrit.cloudera.org:8080/10368
Tested-by: Hao Hao 
Reviewed-by: Dan Burkert 
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_table.cc
2 files changed, 67 insertions(+), 5 deletions(-)

Approvals:
  Hao Hao: Verified
  Dan Burkert: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibe98ce52d6865fa61c0903b38cfac3e86fc05a66
Gerrit-Change-Number: 10368
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [tools] ksck improvements [7/n] Add JSON output option to ksck

[Will Berkeley (Code Review)]

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo,

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

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

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

Change subject: [tools] ksck improvements [7/n] Add JSON output option to ksck
..

[tools] ksck improvements [7/n] Add JSON output option to ksck

This adds support for JSON output to ksck, using a new flag
--ksck_format that supports four options: 'plain_concise',
'plain_full', 'json_pretty', and 'json_concise'.
'plain_concise' formatting is the default: it's the original
formatting and it is not changed by this patch. 'plain_full'
supersedes the --verbose flag. The 'json_pretty' option
pretty-prints a json representation of all the information
gathered by ksck. 'json_compact' ugly-prints the same, and is
suitable for parsing by other programs, like jq.

There's a knock-on effect of how this change is implemented: since
KsckResults is translated to PB and then to JSON via generic PB-to-JSON
code, the printing for CONSENSUS_MISMATCH health has changed to
match its PB stringification. Previously it was stringified as
UNAVAILABLE. Note that it isn't necessarily true that
CONSENSUS_MISMATCH implies unavailable, anyway, since, e.g.
a replica not yet aware of a new leader will cause a
CONSENSUS_MISMATCH state on a tablet that's available.

Here's a sample of the 4 formats run against the same 1-table,
8-tablet cluster:

plain_concise: 
https://gist.github.com/wdberkeley/674a8b0322c4c0ad6e8eb4ef79664d37
plain_full: https://gist.github.com/wdberkeley/841c92cf4e500782b0dc3a30a8c1cbd8
json_pretty: https://gist.github.com/wdberkeley/04dca6dd5ec7a10ad10c4bd30decab35
json_compact: 
https://gist.github.com/wdberkeley/283d48ad248a26073a30e013e19443c3

Change-Id: Ib5da0752f8e41c022611253c300450368f6ae969
---
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck_results.cc
M src/kudu/tools/ksck_results.h
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_cluster.cc
7 files changed, 817 insertions(+), 74 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/10288/10
--
To view, visit http://gerrit.cloudera.org:8080/10288
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib5da0752f8e41c022611253c300450368f6ae969
Gerrit-Change-Number: 10288
Gerrit-PatchSet: 10
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] [tools] ksck improvements [7/n] Add JSON output option to ksck

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10288 )

Change subject: [tools] ksck improvements [7/n] Add JSON output option to ksck
..


Patch Set 9:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/ksck-test.cc
File src/kudu/tools/ksck-test.cc:

http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/ksck-test.cc@453
PS9, Line 453: K
> nit: spacing
Done


http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/ksck-test.cc@489
PS9, Line 489: EXTRACT_ARRAY_CHECK_SIZE(r, cstate, "nonvoter_uuids",
 :  non_voter_uuids, 
ref_non_voter_uuids.size());
> Is there a reason to not do the EXPECT_JSON_STRING_FIELD checks here?
Done


http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/ksck-test.cc@561
PS9, Line 561: "health", KsckCheckResultToString(ref_table.TableStatus()));
 : EXPECT_JSON_INT_FIELD(r, table,
 :  "replication_factor", 
ref_table.replication_factor);
 : EXPECT_JSON_INT_FIELD(r, table,
 :  "total_tablets", 
ref_table.TotalTablets());
 : EXPECT_JSON_INT_FIELD(r, table,
 :  "healthy_tablets", 
ref_table.healthy_tablets);
 : EXPECT_JSON_INT_FIELD(r, table,
 :  "recovering_tablets", 
ref_table.recovering_tablets);
 : EXPECT_JSON_INT_FIELD(r, table,
 :  "underreplicated_tablets", 
ref_table.underreplicated_tablets);
 : EXPECT_JSON_INT_FIELD(r, table,
 :  "unavailable_tablets", 
ref_table.unavailable_tablets);
 : EXPECT_JSON_INT_FIELD(r, table,
 :  "consensus_mismatch_tablets", 
ref_table.consensus_mismatch_tablets);
> nit: spacing
Done


http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/ksck-test.cc@595
PS9, Line 595: "
> nit: spacing
Done


http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/ksck-test.cc@626
PS9, Line 626: "
> nit: spacing
Done


http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/ksck-test.cc@693
PS9, Line 693: std::transform(std::begin(results.error_messages),
 :  std::end(results.error_messages),
 :  std::back_inserter(errors),
 :  [](const Status& s) { return s.ToString(); });
 :   CheckJsonVsErrors(r, "errors", errors);
> nit: you could do away with this transform by passing `vector` to C
Done


http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/ksck-test.cc@1046
PS9, Line 1046: /*consensus_mismatch_tablets=*/ 1,
  :
/*unavailable_tablets=*/ 0));
> nit: whatever fixed this is probably worth calling out in the commit messag
See the "Yowza!" comment.


http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:

http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/ksck_results.cc@133
PS9, Line 133:   return "CONSENSUS_MISMATCH";
> Yowza! I wonder if this should go in a separate patch.
It's forced by the way protobuf stringifies its enum names. We have to match it 
here or do gymnastics just to keep it the same.

I'll call it out in the commit msg.


http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/tool_action_cluster.cc
File src/kudu/tools/tool_action_cluster.cc:

http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/tool_action_cluster.cc@106
PS9, Line 106: .AddOptionalParameter("ksck_format")
> nit: sort order
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5da0752f8e41c022611253c300450368f6ae969
Gerrit-Change-Number: 10288
Gerrit-PatchSet: 9
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 10 May 2018 23:22:17 +
Gerrit-HasComments: Yes

[kudu-CR] Add a column renaming tool

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10368 )

Change subject: Add a column renaming tool
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe98ce52d6865fa61c0903b38cfac3e86fc05a66
Gerrit-Change-Number: 10368
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 10 May 2018 23:06:31 +
Gerrit-HasComments: No

[kudu-CR] [tools] ksck improvements [7/n] Add JSON output option to ksck

[Andrew Wong (Code Review)]

Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10288 )

Change subject: [tools] ksck improvements [7/n] Add JSON output option to ksck
..


Patch Set 9:

(9 comments)

Mostly nits, looks good!

http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/ksck-test.cc
File src/kudu/tools/ksck-test.cc:

http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/ksck-test.cc@453
PS9, Line 453: K
nit: spacing


http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/ksck-test.cc@489
PS9, Line 489: EXTRACT_ARRAY_CHECK_SIZE(r, cstate, "nonvoter_uuids",
 :  non_voter_uuids, 
ref_non_voter_uuids.size());
Is there a reason to not do the EXPECT_JSON_STRING_FIELD checks here?


http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/ksck-test.cc@561
PS9, Line 561: "health", KsckCheckResultToString(ref_table.TableStatus()));
 : EXPECT_JSON_INT_FIELD(r, table,
 :  "replication_factor", 
ref_table.replication_factor);
 : EXPECT_JSON_INT_FIELD(r, table,
 :  "total_tablets", 
ref_table.TotalTablets());
 : EXPECT_JSON_INT_FIELD(r, table,
 :  "healthy_tablets", 
ref_table.healthy_tablets);
 : EXPECT_JSON_INT_FIELD(r, table,
 :  "recovering_tablets", 
ref_table.recovering_tablets);
 : EXPECT_JSON_INT_FIELD(r, table,
 :  "underreplicated_tablets", 
ref_table.underreplicated_tablets);
 : EXPECT_JSON_INT_FIELD(r, table,
 :  "unavailable_tablets", 
ref_table.unavailable_tablets);
 : EXPECT_JSON_INT_FIELD(r, table,
 :  "consensus_mismatch_tablets", 
ref_table.consensus_mismatch_tablets);
nit: spacing


http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/ksck-test.cc@595
PS9, Line 595: "
nit: spacing


http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/ksck-test.cc@626
PS9, Line 626: "
nit: spacing


http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/ksck-test.cc@693
PS9, Line 693: std::transform(std::begin(results.error_messages),
 :  std::end(results.error_messages),
 :  std::back_inserter(errors),
 :  [](const Status& s) { return s.ToString(); });
 :   CheckJsonVsErrors(r, "errors", errors);
nit: you could do away with this transform by passing `vector` to 
CheckJsonVsErrors and doing ref_errors[i].ToString() up at L676. Then we could 
do away with 


http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/ksck-test.cc@1046
PS9, Line 1046: /*consensus_mismatch_tablets=*/ 1,
  :
/*unavailable_tablets=*/ 0));
nit: whatever fixed this is probably worth calling out in the commit message.


http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/ksck_results.cc
File src/kudu/tools/ksck_results.cc:

http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/ksck_results.cc@133
PS9, Line 133:   return "CONSENSUS_MISMATCH";
Yowza! I wonder if this should go in a separate patch.


http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/tool_action_cluster.cc
File src/kudu/tools/tool_action_cluster.cc:

http://gerrit.cloudera.org:8080/#/c/10288/9/src/kudu/tools/tool_action_cluster.cc@106
PS9, Line 106: .AddOptionalParameter("ksck_format")
nit: sort order



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5da0752f8e41c022611253c300450368f6ae969
Gerrit-Change-Number: 10288
Gerrit-PatchSet: 9
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 10 May 2018 23:02:33 +
Gerrit-HasComments: Yes

[kudu-CR] thirdparty: tweak clang compiler flags

[Dan Burkert (Code Review)]

Dan Burkert has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10352 )

Change subject: thirdparty: tweak clang compiler flags
..

thirdparty: tweak clang compiler flags

Prior to this small tweak the thirdparty clang build output thousands of
warnings similar to this one when compiling with clang:

Building CXX object 
lib/Analysis/CMakeFiles/LLVMAnalysis.dir/LazyValueInfo.cpp.o
clang: warning: 
-Wl,-rpath,/Users/dan/src/cpp/kudu/thirdparty/installed/uninstrumented/lib: 
'linker' input unused [-Wunused-command-line-argument]
clang: warning: argument unused during compilation: 
'-L/Users/dan/src/cpp/kudu/thirdparty/installed/uninstrumented/lib' 
[-Wunused-command-line-argument]

Change-Id: Ide4ddbff14d3745c6f2c2f9b14b00da790a6cec6
Reviewed-on: http://gerrit.cloudera.org:8080/10352
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
---
M thirdparty/build-definitions.sh
1 file changed, 16 insertions(+), 13 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ide4ddbff14d3745c6f2c2f9b14b00da790a6cec6
Gerrit-Change-Number: 10352
Gerrit-PatchSet: 6
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-2035 Enable HTTP compression for all webserver's paths

[Fengling Wang (Code Review)]

Fengling Wang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10332 )

Change subject: KUDU-2035 Enable HTTP compression for all webserver's paths
..


Patch Set 4:

(4 comments)

There seems to be no double-compression? So I called 
"http://localhost:8051/tracing/json/end_recording_compressed;, and then got a 
gz file, after I decompressed it, it showed a content that makes sense.

Thanks!

http://gerrit.cloudera.org:8080/#/c/10332/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10332/2//COMMIT_MSG@9
PS2, Line 9: gzip co
> nit: client
Done


http://gerrit.cloudera.org:8080/#/c/10332/2//COMMIT_MSG@10
PS2, Line 10: caller.
> What's wrong with curl in that perspective?
My fault. Cuz a plain curl usually doesn't specify Accept-Encoding. I've 
changed the commit msg.


http://gerrit.cloudera.org:8080/#/c/10332/2/src/kudu/server/webserver.cc
File src/kudu/server/webserver.cc:

http://gerrit.cloudera.org:8080/#/c/10332/2/src/kudu/server/webserver.cc@481
PS2, Line 481: accepte
> nit: accepted by the caller  ?
Done


http://gerrit.cloudera.org:8080/#/c/10332/2/src/kudu/server/webserver.cc@495
PS2, Line 495: "Content-Encoding: gzip\r\n";
> why not just
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7c6db24b0fd2cbcca8a554460d310bd39ee5c071
Gerrit-Change-Number: 10332
Gerrit-PatchSet: 4
Gerrit-Owner: Fengling Wang 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Fengling Wang 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 10 May 2018 22:52:42 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2035 Enable HTTP compression for all webserver's paths

[Fengling Wang (Code Review)]

Hello Will Berkeley, Tidy Bot, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: KUDU-2035 Enable HTTP compression for all webserver's paths
..

KUDU-2035 Enable HTTP compression for all webserver's paths

This patch enables HTTP compression when the gzip compression is
accepted by the caller.

Change-Id: I7c6db24b0fd2cbcca8a554460d310bd39ee5c071
---
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/server/webserver-test.cc
M src/kudu/server/webserver.cc
M src/kudu/util/curl_util.cc
M src/kudu/util/curl_util.h
5 files changed, 90 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/10332/4
--
To view, visit http://gerrit.cloudera.org:8080/10332
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7c6db24b0fd2cbcca8a554460d310bd39ee5c071
Gerrit-Change-Number: 10332
Gerrit-PatchSet: 4
Gerrit-Owner: Fengling Wang 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2035 Enable HTTP compression for all webserver's paths

[Fengling Wang (Code Review)]

Hello Will Berkeley, Tidy Bot, Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: KUDU-2035 Enable HTTP compression for all webserver's paths
..

KUDU-2035 Enable HTTP compression for all webserver's paths

This patch enables HTTP compression when the gzip compression is
enabled from the client request headers.

Change-Id: I7c6db24b0fd2cbcca8a554460d310bd39ee5c071
---
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/server/webserver-test.cc
M src/kudu/server/webserver.cc
M src/kudu/util/curl_util.cc
M src/kudu/util/curl_util.h
5 files changed, 90 insertions(+), 6 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7c6db24b0fd2cbcca8a554460d310bd39ee5c071
Gerrit-Change-Number: 10332
Gerrit-PatchSet: 3
Gerrit-Owner: Fengling Wang 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] Fix int overflow GetClockTimeMicros() on macOS

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10371 )

Change subject: Fix int overflow GetClockTimeMicros() on macOS
..

Fix int overflow GetClockTimeMicros() on macOS

On macOS mach_timespec_t.tv_sec is only 4 bytes and we
were converting to micros before moving to a bigger var.
This would cause all the wall times obtained on a mac (through
GetClockTimeMicros() to be wrong.

This was likely the cause of KUDU-2435 and KUDU-2408 too, since
the time would easily wrap, causing us to a update the clock with
a value that was seemingly from the future.

Posix just requires it to be an integer
(see: https://en.wikipedia.org/w/index.php?title=Time_t=450752800)
so also fixed it on the non-macOS path.

Testing this is likely not worth it since the only real change
was on macOS where the overlow doesn't happen anymore.

Change-Id: Ie0eaa548f61352be529755a732566613cfa72098
Reviewed-on: http://gerrit.cloudera.org:8080/10371
Reviewed-by: Todd Lipcon 
Reviewed-by: Dan Burkert 
Reviewed-by: Grant Henke 
Tested-by: Kudu Jenkins
---
M src/kudu/gutil/walltime.h
1 file changed, 13 insertions(+), 2 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, but someone else must approve
  Dan Burkert: Looks good to me, approved
  Grant Henke: Looks good to me, approved
  Kudu Jenkins: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie0eaa548f61352be529755a732566613cfa72098
Gerrit-Change-Number: 10371
Gerrit-PatchSet: 3
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-2426 Fix WRONG SERVER UUID case in ksck

[Andrew Wong (Code Review)]

Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10293 )

Change subject: KUDU-2426 Fix WRONG_SERVER_UUID case in ksck
..


Patch Set 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/10293/4/src/kudu/tools/ksck-test.cc
File src/kudu/tools/ksck-test.cc:

http://gerrit.cloudera.org:8080/#/c/10293/4/src/kudu/tools/ksck-test.cc@477
PS4, Line 477: TEST_F(KsckTest, TestWrongUUIDTabletServer) {
Hmm.. maybe also add a regression test specific to KUDU-2426 that other remote 
errors don't end up with WRONG_SERVER_UUID?

Also it'd be nice if we had an integration test that actually tested the 
RemoteTabletServer's behavior, maybe using the ClusterVerifier and a mini 
cluster? TestRestartWithDifferentUUID in raft_consensus-itest.cc might be worth 
checking out.


http://gerrit.cloudera.org:8080/#/c/10293/4/src/kudu/tools/ksck.h
File src/kudu/tools/ksck.h:

http://gerrit.cloudera.org:8080/#/c/10293/4/src/kudu/tools/ksck.h@276
PS4, Line 276: // Connects to the configured tablet server and populates the 
fields of this class.
> Document if 'health' is allowed to be nullptr, and if it is, what the seman
Does returning a non-OK status mean that health is guaranteed to not be 
HEALTHY? Similarly, if this returns OK, does that mean health is guaranteed to 
be HEALTHY? I think it'd be nice if both of these were true, although 
guaranteeing them might take some work.

If not, I at least agree with Will, the new semantics of this function should 
be documented.


http://gerrit.cloudera.org:8080/#/c/10293/2/src/kudu/tools/ksck_remote.cc
File src/kudu/tools/ksck_remote.cc:

http://gerrit.cloudera.org:8080/#/c/10293/2/src/kudu/tools/ksck_remote.cc@165
PS2, Line 165: if (response_uuid != uuid()) {
> We have an expected uuid known ahead of time, from an authority, for each t
Ah, I meant changing the signature of the KsckMaster::FetchInfo() to also 
return server health (ie UNAVAILABLE). But I agree, it belongs in a separate 
patch if anything.


http://gerrit.cloudera.org:8080/#/c/10293/4/src/kudu/tools/ksck_remote.cc
File src/kudu/tools/ksck_remote.cc:

http://gerrit.cloudera.org:8080/#/c/10293/4/src/kudu/tools/ksck_remote.cc@166
PS4, Line 166: *health = KsckServerHealth::WRONG_SERVER_UUID;
Why don't we return an error here anymore?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b4f50fe4dd94450b4f2e34dbad315bd761b071f
Gerrit-Change-Number: 10293
Gerrit-PatchSet: 4
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 10 May 2018 22:19:18 +
Gerrit-HasComments: Yes

[kudu-CR] Fix int overflow GetClockTimeMicros() on macOS

[Grant Henke (Code Review)]

Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10371 )

Change subject: Fix int overflow GetClockTimeMicros() on macOS
..


Patch Set 2: Code-Review+2

This passes my failing Java tests on mac.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0eaa548f61352be529755a732566613cfa72098
Gerrit-Change-Number: 10371
Gerrit-PatchSet: 2
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 10 May 2018 22:04:20 +
Gerrit-HasComments: No

[kudu-CR] Fix int overflow GetClockTimeMicros() on macOS

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10371 )

Change subject: Fix int overflow GetClockTimeMicros() on macOS
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0eaa548f61352be529755a732566613cfa72098
Gerrit-Change-Number: 10371
Gerrit-PatchSet: 2
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 10 May 2018 22:03:21 +
Gerrit-HasComments: No

[kudu-CR] Fix int overflow GetClockTimeMicros() on macOS

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10371 )

Change subject: Fix int overflow GetClockTimeMicros() on macOS
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0eaa548f61352be529755a732566613cfa72098
Gerrit-Change-Number: 10371
Gerrit-PatchSet: 2
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 10 May 2018 21:54:55 +
Gerrit-HasComments: No

[kudu-CR] Fix int overflow GetClockTimeMicros() on macOS

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10371 )

Change subject: Fix int overflow GetClockTimeMicros() on macOS
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10371/1/src/kudu/gutil/walltime.h
File src/kudu/gutil/walltime.h:

http://gerrit.cloudera.org:8080/#/c/10371/1/src/kudu/gutil/walltime.h@87
PS1, Line 87:   micros_from_secs *= 1000 * 1000 + ts.tv_nsec / 1000;
> Is the precedence here correct?  Seems like it's now
Done


http://gerrit.cloudera.org:8080/#/c/10371/1/src/kudu/gutil/walltime.h@141
PS1, Line 141:   micros_from_secs *= 1000 * 1000 + ts.tv_nsec / 1000;
> same
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0eaa548f61352be529755a732566613cfa72098
Gerrit-Change-Number: 10371
Gerrit-PatchSet: 1
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 10 May 2018 21:49:21 +
Gerrit-HasComments: Yes

[kudu-CR] Fix int overflow GetClockTimeMicros() on macOS

[David Ribeiro Alves (Code Review)]

Hello Dan Burkert, Kudu Jenkins, Grant Henke, Todd Lipcon,

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

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

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

Change subject: Fix int overflow GetClockTimeMicros() on macOS
..

Fix int overflow GetClockTimeMicros() on macOS

On macOS mach_timespec_t.tv_sec is only 4 bytes and we
were converting to micros before moving to a bigger var.
This would cause all the wall times obtained on a mac (through
GetClockTimeMicros() to be wrong.

This was likely the cause of KUDU-2435 and KUDU-2408 too, since
the time would easily wrap, causing us to a update the clock with
a value that was seemingly from the future.

Posix just requires it to be an integer
(see: https://en.wikipedia.org/w/index.php?title=Time_t=450752800)
so also fixed it on the non-macOS path.

Testing this is likely not worth it since the only real change
was on macOS where the overlow doesn't happen anymore.

Change-Id: Ie0eaa548f61352be529755a732566613cfa72098
---
M src/kudu/gutil/walltime.h
1 file changed, 13 insertions(+), 2 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0eaa548f61352be529755a732566613cfa72098
Gerrit-Change-Number: 10371
Gerrit-PatchSet: 2
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Fix int overflow GetClockTimeMicros() on macOS

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10371 )

Change subject: Fix int overflow GetClockTimeMicros() on macOS
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10371/1/src/kudu/gutil/walltime.h
File src/kudu/gutil/walltime.h:

http://gerrit.cloudera.org:8080/#/c/10371/1/src/kudu/gutil/walltime.h@87
PS1, Line 87:   micros_from_secs *= 1000 * 1000 + ts.tv_nsec / 1000;
> Is the precedence here correct?  Seems like it's now
ah, good point, changed it just before I pushed to have one less line.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0eaa548f61352be529755a732566613cfa72098
Gerrit-Change-Number: 10371
Gerrit-PatchSet: 1
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 10 May 2018 21:46:53 +
Gerrit-HasComments: Yes

[kudu-CR] thirdparty: tweak clang compiler flags

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10352 )

Change subject: thirdparty: tweak clang compiler flags
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide4ddbff14d3745c6f2c2f9b14b00da790a6cec6
Gerrit-Change-Number: 10352
Gerrit-PatchSet: 5
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 10 May 2018 21:24:31 +
Gerrit-HasComments: No

[kudu-CR] thirdparty: tweak clang compiler flags

[Dan Burkert (Code Review)]

Hello Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: thirdparty: tweak clang compiler flags
..

thirdparty: tweak clang compiler flags

Prior to this small tweak the thirdparty clang build output thousands of
warnings similar to this one when compiling with clang:

Building CXX object 
lib/Analysis/CMakeFiles/LLVMAnalysis.dir/LazyValueInfo.cpp.o
clang: warning: 
-Wl,-rpath,/Users/dan/src/cpp/kudu/thirdparty/installed/uninstrumented/lib: 
'linker' input unused [-Wunused-command-line-argument]
clang: warning: argument unused during compilation: 
'-L/Users/dan/src/cpp/kudu/thirdparty/installed/uninstrumented/lib' 
[-Wunused-command-line-argument]

Change-Id: Ide4ddbff14d3745c6f2c2f9b14b00da790a6cec6
---
M thirdparty/build-definitions.sh
1 file changed, 16 insertions(+), 13 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ide4ddbff14d3745c6f2c2f9b14b00da790a6cec6
Gerrit-Change-Number: 10352
Gerrit-PatchSet: 5
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] thirdparty: tweak clang compiler flags

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10352 )

Change subject: thirdparty: tweak clang compiler flags
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10352/4/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

http://gerrit.cloudera.org:8080/#/c/10352/4/thirdparty/build-definitions.sh@280
PS4, Line 280: thirdpart
> thirdparty
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide4ddbff14d3745c6f2c2f9b14b00da790a6cec6
Gerrit-Change-Number: 10352
Gerrit-PatchSet: 5
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 10 May 2018 21:19:44 +
Gerrit-HasComments: Yes

[kudu-CR] Fix int overflow GetClockTimeMicros() on macOS

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10371 )

Change subject: Fix int overflow GetClockTimeMicros() on macOS
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10371/1/src/kudu/gutil/walltime.h
File src/kudu/gutil/walltime.h:

http://gerrit.cloudera.org:8080/#/c/10371/1/src/kudu/gutil/walltime.h@87
PS1, Line 87:   micros_from_secs *= 1000 * 1000 + ts.tv_nsec / 1000;
Is the precedence here correct?  Seems like it's now

MicrosecondsInt64 micros_from_secs = static_cast(ts.tv_sec) 
* ((1000 * 1000) + (ts.tvnsec / 1000))

which isn't right.  Regardless of whether it's right, the *= with + is 
confusing, so maybe split it out.


http://gerrit.cloudera.org:8080/#/c/10371/1/src/kudu/gutil/walltime.h@141
PS1, Line 141:   micros_from_secs *= 1000 * 1000 + ts.tv_nsec / 1000;
same



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0eaa548f61352be529755a732566613cfa72098
Gerrit-Change-Number: 10371
Gerrit-PatchSet: 1
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 10 May 2018 21:09:42 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2287 Expose election failures as metrics

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10076 )

Change subject: KUDU-2287 Expose election failures as metrics
..


Patch Set 13:

(2 comments)

Approach looks good. Needs a test for each metric.

http://gerrit.cloudera.org:8080/#/c/10076/13/src/kudu/consensus/raft_consensus.h
File src/kudu/consensus/raft_consensus.h:

http://gerrit.cloudera.org:8080/#/c/10076/13/src/kudu/consensus/raft_consensus.h@868
PS13, Line 868: time_since_stable_leader_
This member is supposed to represent the moment in time when this replica 
concluded there's no longer a leader, while there's no leader. This means the 
name is confusing because it sounds like it is a duration. Howabout 
"time_stable_leader_lost" or something?

Additionally, during periods of stable leadership there is no proper value for 
this member. I think you have been using MonoTime::Min() as a sentinel for 
this; perhaps use a boost::optional so the types more closely mirror 
the logic?


http://gerrit.cloudera.org:8080/#/c/10076/13/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:

http://gerrit.cloudera.org:8080/#/c/10076/13/src/kudu/consensus/raft_consensus.cc@162
PS13, Line 162: "The time elapsed without successful election since majority 
was "
  :   "lost in nanoseconds."
This description is confusing to me. IIUC, this metric is the amount of time in 
nanoseconds since this node detected a failure of the leader, or 0 is a leader 
is believed alive?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b25df258cdba7bdae7bb2d7b4eb3d73b53425c3
Gerrit-Change-Number: 10076
Gerrit-PatchSet: 13
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 10 May 2018 21:02:34 +
Gerrit-HasComments: Yes

[kudu-CR] Fix int overflow GetClockTimeMicros() on macOS

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10371


Change subject: Fix int overflow GetClockTimeMicros() on macOS
..

Fix int overflow GetClockTimeMicros() on macOS

On macOS mach_timespec_t.tv_sec is only 4 bytes and we
were converting to micros before moving to a bigger var.
This would cause all the wall times obtained on a mac (through
GetClockTimeMicros() to be wrong.

This was likely the cause of KUDU-2435 and KUDU-2408 too, since
the time would easily wrap, causing us to a update the clock with
a value that was seemingly from the future.

Posix just requires it to be an integer
(see: https://en.wikipedia.org/w/index.php?title=Time_t=450752800)
so also fixed it on the non-macOS path.

Testing this is likely not worth it since the only real change
was on macOS where the overlow doesn't happen anymore.

Change-Id: Ie0eaa548f61352be529755a732566613cfa72098
---
M src/kudu/gutil/walltime.h
1 file changed, 11 insertions(+), 2 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie0eaa548f61352be529755a732566613cfa72098
Gerrit-Change-Number: 10371
Gerrit-PatchSet: 1
Gerrit-Owner: David Ribeiro Alves 

[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9526 )

Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
..


Patch Set 13:

> > Unrelated flaky test.
 >
 > Did you try to run it against some test cluster already?
 >
 > In other words, do you mind sharing details on how you test these
 > changes?  Thanks!

Yeah, I test it with a cluster. The sets model is included in Kudu test as 
default. So just follow the steps in README.adoc of kudu-jepsen should be fine.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 13
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 10 May 2018 20:53:33 +
Gerrit-HasComments: No

[kudu-CR] [tools] Add ids to tables and tablets; table name to tablets

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10369 )

Change subject: [tools] Add ids to tables and tablets; table name to tablets
..


Patch Set 1:

Known + unrelated failure.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I19cd76a4c4c59c28930a44c0593021039903bec1
Gerrit-Change-Number: 10369
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 10 May 2018 20:07:32 +
Gerrit-HasComments: No

[kudu-CR] [tools] Add ids to tables and tablets; table name to tablets

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10369 )

Change subject: [tools] Add ids to tables and tablets; table name to tablets
..


Patch Set 1: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I19cd76a4c4c59c28930a44c0593021039903bec1
Gerrit-Change-Number: 10369
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 10 May 2018 20:07:39 +
Gerrit-HasComments: No

[kudu-CR] [tools] Add ids to tables and tablets; table name to tablets

[Will Berkeley (Code Review)]

Will Berkeley has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10369 )

Change subject: [tools] Add ids to tables and tablets; table name to tablets
..

[tools] Add ids to tables and tablets; table name to tablets

This adds tablet id, table id, and table name to KsckTabletSummary,
and table id to KsckTableSummary.

This will be tested in the follow-up JSON formatting patch.

Change-Id: I19cd76a4c4c59c28930a44c0593021039903bec1
Reviewed-on: http://gerrit.cloudera.org:8080/10369
Reviewed-by: Alexey Serbin 
Tested-by: Will Berkeley 
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_results.h
5 files changed, 24 insertions(+), 6 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved
  Will Berkeley: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I19cd76a4c4c59c28930a44c0593021039903bec1
Gerrit-Change-Number: 10369
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] [tools] Add ids to tables and tablets; table name to tablets

[Will Berkeley (Code Review)]

Will Berkeley has removed a vote on this change.

Change subject: [tools] Add ids to tables and tablets; table name to tablets
..


Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/10369
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I19cd76a4c4c59c28930a44c0593021039903bec1
Gerrit-Change-Number: 10369
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [docs] Update macOS build docs

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10356 )

Change subject: [docs] Update macOS build docs
..


Patch Set 2:

> > Patch Set 2:
 > >
 > > > > Patch Set 2:
 > >  > >
 > >  > > Is this relevant after 60c8e305bef9ace8688aabbf18e2f500e8d9135c
 > ?
 > >  >
 > >  > probably less relevant than the original patch set, but I
 > believe
 > >  > it could still fail, at least I managed to get this failure
 > even
 > >  > without the check_openssl in preflight before adding
 > >  > PKG_CONFIG_PATH to my .bash_profile. I'm not completely sure
 > how
 > >  > that happened though.
 > >
 > > If you use preflight and your openssl instaled via HomeBrew, then
 > I would expect it to fail regardless of PKG_CONFIG_PATH in
 > .bash_profile b prior 60c8e30 changelist.
 > >
 > > However, I don't think this is relevant after 60c8e30.  I would
 > prefer to keep less details in the docs if the things are already
 > automated.
 >
 > +1.  The dupes removal is still relevant.

Yep, I also think it's worth removing homebrew/dupes; that makes sense.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2a8aa7f58b7401115f36f42634d80b4d64e9794
Gerrit-Change-Number: 10356
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 10 May 2018 20:00:54 +
Gerrit-HasComments: No

[kudu-CR] [tools] minor enhancements on 'kudu cluster ksck' output

[Alexey Serbin (Code Review)]

Alexey Serbin has abandoned this change. ( 
http://gerrit.cloudera.org:8080/10054 )

Change subject: [tools] minor enhancements on 'kudu cluster ksck' output
..


Abandoned

Will added corresponding functionality with 0355d373a, so this patch is no 
longer needed.
--
To view, visit http://gerrit.cloudera.org:8080/10054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I0888ef640b215e1e0cf1f872f02cfe34f4ef5ba6
Gerrit-Change-Number: 10054
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] [tools] minor enhancements on 'kudu cluster ksck' output

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10054 )

Change subject: [tools] minor enhancements on 'kudu cluster ksck' output
..


Patch Set 6:

> After chatting with Alexey I added these as part of the "6/n" patch
 > 0355d373a.
 >
 > Alexey, think we can abandon this change now?

 > After chatting with Alexey I added these as part of the "6/n" patch
 > 0355d373a.
 >
 > Alexey, think we can abandon this change now?

Ah, yes -- I think this patch is no longer needed.  Thank you for the heads up!


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0888ef640b215e1e0cf1f872f02cfe34f4ef5ba6
Gerrit-Change-Number: 10054
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 10 May 2018 19:50:19 +
Gerrit-HasComments: No

[kudu-CR] A new Jepsen checker for READ YOUR WRITES scan mode

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9526 )

Change subject: A new Jepsen checker for READ_YOUR_WRITES scan mode
..


Patch Set 13:

> Unrelated flaky test.

Did you try to run it against some test cluster already?

In other words, do you mind sharing details on how you test these changes?  
Thanks!


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I92d5c0e3b91af58576eb6cd408d922ec7c0fef6c
Gerrit-Change-Number: 9526
Gerrit-PatchSet: 13
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 10 May 2018 19:43:22 +
Gerrit-HasComments: No

[kudu-CR] [docs] Update macOS build docs

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10356 )

Change subject: [docs] Update macOS build docs
..


Patch Set 2:

> Patch Set 2:
>
> > > Patch Set 2:
>  > >
>  > > Is this relevant after 60c8e305bef9ace8688aabbf18e2f500e8d9135c ?
>  >
>  > probably less relevant than the original patch set, but I believe
>  > it could still fail, at least I managed to get this failure even
>  > without the check_openssl in preflight before adding
>  > PKG_CONFIG_PATH to my .bash_profile. I'm not completely sure how
>  > that happened though.
>
> If you use preflight and your openssl instaled via HomeBrew, then I would 
> expect it to fail regardless of PKG_CONFIG_PATH in .bash_profile b prior 
> 60c8e30 changelist.
>
> However, I don't think this is relevant after 60c8e30.  I would prefer to 
> keep less details in the docs if the things are already automated.

+1.  The dupes removal is still relevant.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2a8aa7f58b7401115f36f42634d80b4d64e9794
Gerrit-Change-Number: 10356
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 10 May 2018 19:43:23 +
Gerrit-HasComments: No

[kudu-CR] [docs] Update macOS build docs

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10356 )

Change subject: [docs] Update macOS build docs
..


Patch Set 2:

> > Patch Set 2:
 > >
 > > Is this relevant after 60c8e305bef9ace8688aabbf18e2f500e8d9135c ?
 >
 > probably less relevant than the original patch set, but I believe
 > it could still fail, at least I managed to get this failure even
 > without the check_openssl in preflight before adding
 > PKG_CONFIG_PATH to my .bash_profile. I'm not completely sure how
 > that happened though.

If you use preflight and your openssl instaled via HomeBrew, then I would 
expect it to fail regardless of PKG_CONFIG_PATH in .bash_profile b prior 
60c8e30 changelist.

However, I don't think this is relevant after 60c8e30.  I would prefer to keep 
less details in the docs if the things are already automated.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2a8aa7f58b7401115f36f42634d80b4d64e9794
Gerrit-Change-Number: 10356
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 10 May 2018 19:41:36 +
Gerrit-HasComments: No

[kudu-CR] [tools] Add ids to tables and tablets; table name to tablets

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10369 )

Change subject: [tools] Add ids to tables and tablets; table name to tablets
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I19cd76a4c4c59c28930a44c0593021039903bec1
Gerrit-Change-Number: 10369
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 10 May 2018 19:20:19 +
Gerrit-HasComments: No

[kudu-CR] Add a column renaming tool

[Hao Hao (Code Review)]

Hao Hao has removed a vote on this change.

Change subject: Add a column renaming tool
..


Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/10368
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ibe98ce52d6865fa61c0903b38cfac3e86fc05a66
Gerrit-Change-Number: 10368
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Add a column renaming tool

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10368 )

Change subject: Add a column renaming tool
..


Patch Set 1: Verified+1

Unrelated flaky test.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe98ce52d6865fa61c0903b38cfac3e86fc05a66
Gerrit-Change-Number: 10368
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 10 May 2018 19:16:49 +
Gerrit-HasComments: No

[kudu-CR] [flags] run validators after processing help flags

[Alexey Serbin (Code Review)]

Alexey Serbin has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10342 )

Change subject: [flags] run validators after processing help flags
..

[flags] run validators after processing help flags

Run flag validators (both individual and group ones) after
processing the help flags.

Also, this is a follow-up for 297b72bd26cdd546f0a73cda7487c80566388492:
the default value for the --force_block_cache_capacity flag is set to
'true' everywhere but in master and tablet server.  Otherwise,
corresponding group validator could strike while running binaries
that link the cfile library (e.g. the kudu CLI tool) at machines
with less than 256 MB of available memory.

Change-Id: I51f263890c91251f0d13c826c40dfd20fe284b59
Reviewed-on: http://gerrit.cloudera.org:8080/10342
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
---
M src/kudu/cfile/block_cache.cc
M src/kudu/consensus/CMakeLists.txt
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/master/master_main.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/tablet_server_main.cc
M src/kudu/util/flags.cc
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
10 files changed, 50 insertions(+), 42 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I51f263890c91251f0d13c826c40dfd20fe284b59
Gerrit-Change-Number: 10342
Gerrit-PatchSet: 9
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] [tools] ksck improvements [7/n] Add JSON output option to ksck

[Will Berkeley (Code Review)]

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo,

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

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

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

Change subject: [tools] ksck improvements [7/n] Add JSON output option to ksck
..

[tools] ksck improvements [7/n] Add JSON output option to ksck

This adds support for JSON output to ksck, using a new flag
--ksck_format that supports four options: 'plain_concise',
'plain_full', 'json_pretty', and 'json_concise'.
'plain_concise' formatting is the default: it's the original
formatting and it is not changed by this patch. 'plain_full'
supersedes the --verbose flag. The 'json_pretty' option
pretty-prints a json representation of all the information
gathered by ksck. 'json_compact' ugly-prints the same, and is
suitable for parsing by other programs, like jq.

Here's a sample of the 4 formats run against the same 1-table,
8-tablet cluster:

plain_concise: 
https://gist.github.com/wdberkeley/674a8b0322c4c0ad6e8eb4ef79664d37
plain_full: https://gist.github.com/wdberkeley/841c92cf4e500782b0dc3a30a8c1cbd8
json_pretty: https://gist.github.com/wdberkeley/04dca6dd5ec7a10ad10c4bd30decab35
json_compact: 
https://gist.github.com/wdberkeley/283d48ad248a26073a30e013e19443c3

Change-Id: Ib5da0752f8e41c022611253c300450368f6ae969
---
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck_results.cc
M src/kudu/tools/ksck_results.h
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_cluster.cc
7 files changed, 820 insertions(+), 74 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib5da0752f8e41c022611253c300450368f6ae969
Gerrit-Change-Number: 10288
Gerrit-PatchSet: 9
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] [tools] ksck improvements [7/n] Add JSON output option to ksck

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10288 )

Change subject: [tools] ksck improvements [7/n] Add JSON output option to ksck
..


Patch Set 8:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/10288/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10288/8//COMMIT_MSG@21
PS8, Line 21: 
https://gist.github.com/wdberkeley/0b4a4e38a9bd85acdaec627782217794
> nit: Would be nice to see the other formats, even if they're not as readabl
Done


http://gerrit.cloudera.org:8080/#/c/10288/8/src/kudu/tools/ksck-test.cc
File src/kudu/tools/ksck-test.cc:

http://gerrit.cloudera.org:8080/#/c/10288/8/src/kudu/tools/ksck-test.cc@377
PS8, Line 377: CHECK
> nit: mind renaming these to EXPECT_* or VERIFY* or something? CHECK_* makes
Done. I'm keeping the name 'value' though, since it's the rapidjson::Value* 
from which the field value will be extracted and that name helped me remember 
the order when I was writing the code.


http://gerrit.cloudera.org:8080/#/c/10288/8/src/kudu/tools/ksck_results.h
File src/kudu/tools/ksck_results.h:

http://gerrit.cloudera.org:8080/#/c/10288/8/src/kudu/tools/ksck_results.h@279
PS8, Line 279: // Print this KsckResults to 'out', according to the PrintMode 
'mode'.
 :   Status PrintTo(PrintMode mode, std::ostream& out);
 :
 :   // Print this KsckResults to 'out' in json format.
 :   // 'mode' must be PrintMode::JSON_PRETTY or 
PrintMode::JSON_COMPACT.
 :   Status PrintJsonTo(std::ostream& out, PrintMode mode) const;
> nit: maybe keep the ordering the same? Also not the fault of this patch, bu
Done.

Ideally PrintTo() would be const but we sort various pieces of information in 
order to pretty-print it. I opted to make it non-const instead of making lots 
of copies.


http://gerrit.cloudera.org:8080/#/c/10288/8/src/kudu/tools/ksck_results.h@318
PS8, Line 318: // Methods for writing components of KsckResults as json.
 : void ConsensusStateToJson(const KsckConsensusState& cstate, 
JsonWriter* w);
 : void MasterConsensusStatesToJson(const KsckConsensusStateMap& 
master_cstates,
 :  bool conflict,
 :  JsonWriter* w);
 : void ServerHealthSummariesToJson(KsckServerType type,
 :  const 
std::vector& summaries,
 :  JsonWriter* w);
 : void TableSummariesToJson(const std::vector& 
table_summaries,
 :   JsonWriter* w);
 : void TabletSummariesToJson(const std::vector& 
tablet_summaries,
 :JsonWriter* w);
 : void ChecksumResultsToJson(const KsckChecksumResults& 
checksum_results,
 :JsonWriter* w);
 : void TotalCountsToJson(const KsckResults& results,
 :JsonWriter* w);
> I might be missing something. Are these somewhere? I can't find them in ksc
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib5da0752f8e41c022611253c300450368f6ae969
Gerrit-Change-Number: 10288
Gerrit-PatchSet: 8
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 10 May 2018 18:57:19 +
Gerrit-HasComments: Yes

[kudu-CR] [tools] Add ids to tables and tablets; table name to tablets

[Will Berkeley (Code Review)]

Will Berkeley has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10369


Change subject: [tools] Add ids to tables and tablets; table name to tablets
..

[tools] Add ids to tables and tablets; table name to tablets

This adds tablet id, table id, and table name to KsckTabletSummary,
and table id to KsckTableSummary.

This will be tested in the follow-up JSON formatting patch.

Change-Id: I19cd76a4c4c59c28930a44c0593021039903bec1
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_results.h
5 files changed, 24 insertions(+), 6 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I19cd76a4c4c59c28930a44c0593021039903bec1
Gerrit-Change-Number: 10369
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 

[kudu-CR] KUDU-2426 Fix WRONG SERVER UUID case in ksck

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10293 )

Change subject: KUDU-2426 Fix WRONG_SERVER_UUID case in ksck
..


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/10293/4/src/kudu/tools/ksck-test.cc
File src/kudu/tools/ksck-test.cc:

http://gerrit.cloudera.org:8080/#/c/10293/4/src/kudu/tools/ksck-test.cc@101
PS4, Line 101: 345;
Just for code hygiene, could you add a CHECK(health) here to ensure health is 
not nullptr? Alternatively, the function could not set the output parameter if 
nullptr is passed.


http://gerrit.cloudera.org:8080/#/c/10293/4/src/kudu/tools/ksck-test.cc@129
PS4, Line 129:
nit: "these variables".


http://gerrit.cloudera.org:8080/#/c/10293/4/src/kudu/tools/ksck.h
File src/kudu/tools/ksck.h:

http://gerrit.cloudera.org:8080/#/c/10293/4/src/kudu/tools/ksck.h@276
PS4, Line 276: // Connects to the configured tablet server and populates the 
fields of this class.
Document if 'health' is allowed to be nullptr, and if it is, what the semantics 
are in that case (probably, it is just not set as an output param in that case).


http://gerrit.cloudera.org:8080/#/c/10293/2/src/kudu/tools/ksck_remote.cc
File src/kudu/tools/ksck_remote.cc:

http://gerrit.cloudera.org:8080/#/c/10293/2/src/kudu/tools/ksck_remote.cc@165
PS2, Line 165:   return Status::UuidMismatch(Substitute("ID reported by 
tablet server ($0) doesn't "
> thanks for the tip. as for the masters, it's a good idea, but I'd rather do
We have an expected uuid known ahead of time, from an authority, for each 
tserver- the uuid known to the leader master. We do not have authoritative 
uuids for the masters, just the addresses from the command line, so it's 
tricker to claim a master has the wrong uuid. The test that comes to mind is 
that if a leader master expected UUID A but the master reports UUID B, it's an 
imposter. We can't do that check in FetchInfo. We also can't do it if there's 
no leader master.

Agree the WRONG_UUID_LOGIC for masters should be another patch.


http://gerrit.cloudera.org:8080/#/c/10293/4/src/kudu/tools/ksck_remote.cc
File src/kudu/tools/ksck_remote.cc:

http://gerrit.cloudera.org:8080/#/c/10293/4/src/kudu/tools/ksck_remote.cc@155
PS4, Line 155:
Also need to handle the nullptr case.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b4f50fe4dd94450b4f2e34dbad315bd761b071f
Gerrit-Change-Number: 10293
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 10 May 2018 18:24:01 +
Gerrit-HasComments: Yes

[kudu-CR] [docs] Update macOS build docs

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10356 )

Change subject: [docs] Update macOS build docs
..


Patch Set 2:

(2 comments)

> Patch Set 2:
>
> > Patch Set 2:
> >
> > Is this relevant after 60c8e305bef9ace8688aabbf18e2f500e8d9135c ?
>
> probably less relevant than the original patch set, but I believe it could 
> still fail, at least I managed to get this failure even without the 
> check_openssl in preflight before adding PKG_CONFIG_PATH to my .bash_profile. 
> I'm not completely sure how that happened though.

FWIW I haven't had to set PKG_CONFIG_PATH to get the build to work

http://gerrit.cloudera.org:8080/#/c/10356/2/docs/installation.adoc
File docs/installation.adoc:

http://gerrit.cloudera.org:8080/#/c/10356/2/docs/installation.adoc@615
PS2, Line 615: libraris
nit: libraries


http://gerrit.cloudera.org:8080/#/c/10356/2/docs/installation.adoc@617
PS2, Line 617: /usr/local
surround with back ticks



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2a8aa7f58b7401115f36f42634d80b4d64e9794
Gerrit-Change-Number: 10356
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 10 May 2018 18:21:11 +
Gerrit-HasComments: Yes

[kudu-CR] Add a column renaming tool

[Hao Hao (Code Review)]

Hao Hao has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10368


Change subject: Add a column renaming tool
..

Add a column renaming tool

This commit introduces a tool to rename a table's column. Similar to
table renaming tool, when HMS integration feature is enabled, users can
use this tool to rename table's column that have hive incompatible
names. In order to allow these tables to be upgraded.

Change-Id: Ibe98ce52d6865fa61c0903b38cfac3e86fc05a66
---
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_table.cc
2 files changed, 67 insertions(+), 5 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibe98ce52d6865fa61c0903b38cfac3e86fc05a66
Gerrit-Change-Number: 10368
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao 

[kudu-CR] [tools] minor enhancements on 'kudu cluster ksck' output

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10054 )

Change subject: [tools] minor enhancements on 'kudu cluster ksck' output
..


Patch Set 6:

After chatting with Alexey I added these as part of the "6/n" patch 0355d373a.

Alexey, think we can abandon this change now?


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0888ef640b215e1e0cf1f872f02cfe34f4ef5ba6
Gerrit-Change-Number: 10054
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 10 May 2018 17:56:01 +
Gerrit-HasComments: No

[kudu-CR] [flags] run validators after processing help flags

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10342 )

Change subject: [flags] run validators after processing help flags
..


Patch Set 8: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I51f263890c91251f0d13c826c40dfd20fe284b59
Gerrit-Change-Number: 10342
Gerrit-PatchSet: 8
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 10 May 2018 17:34:43 +
Gerrit-HasComments: No

[kudu-CR] thirdparty: tweak clang compiler flags

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10352 )

Change subject: thirdparty: tweak clang compiler flags
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10352/4/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

http://gerrit.cloudera.org:8080/#/c/10352/4/thirdparty/build-definitions.sh@280
PS4, Line 280: thirparty
thirdparty



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide4ddbff14d3745c6f2c2f9b14b00da790a6cec6
Gerrit-Change-Number: 10352
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 10 May 2018 17:33:54 +
Gerrit-HasComments: Yes

[kudu-CR] [flags] run validators after processing help flags

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10342 )

Change subject: [flags] run validators after processing help flags
..


Patch Set 7:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/cfile/block_cache.cc
File src/kudu/cfile/block_cache.cc:

http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/cfile/block_cache.cc@45
PS7, Line 45: // For details, see https://gerrit.cloudera.org/#/c/10342/
> You can omit this; it'll be in the git commit message anyway.
I think it's worth keeping the comment about the default value set to 'true', 
at least to keep readers of the code less confused.  Indeed, adding gerrit 
review item information was redundant; it's removed.


http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/master/master_main.cc
File src/kudu/master/master_main.cc:

http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/master/master_main.cc@58
PS7, Line 58: to
> omit
Done


http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/master/master_main.cc@60
PS7, Line 60:   SetCommandLineOptionWithMode("force_block_cache_capacity", 
"false",
> Check that the return value is not empty.
I was following the most common notation of using this method in the Kudu code; 
indeed, that's not safe.


http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/tools/CMakeLists.txt
File src/kudu/tools/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/tools/CMakeLists.txt@18
PS7, Line 18: set(LINK_LIBS
> This is only used in kudu_tools_util, so maybe just merge it in there?
Done


http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/tools/CMakeLists.txt@61
PS7, Line 61:   server_process
> Sort order.
Done


http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/tserver/tablet_server_main.cc
File src/kudu/tserver/tablet_server_main.cc:

http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/tserver/tablet_server_main.cc@63
PS7, Line 63: to
> omit
Done


http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/tserver/tablet_server_main.cc@65
PS7, Line 65:   SetCommandLineOptionWithMode("force_block_cache_capacity", 
"false",
> Check return value.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I51f263890c91251f0d13c826c40dfd20fe284b59
Gerrit-Change-Number: 10342
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 10 May 2018 17:21:01 +
Gerrit-HasComments: Yes

[kudu-CR] [flags] run validators after processing help flags

[Alexey Serbin (Code Review)]

Hello Will Berkeley, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, Todd Lipcon,

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

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

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

Change subject: [flags] run validators after processing help flags
..

[flags] run validators after processing help flags

Run flag validators (both individual and group ones) after
processing the help flags.

Also, this is a follow-up for 297b72bd26cdd546f0a73cda7487c80566388492:
the default value for the --force_block_cache_capacity flag is set to
'true' everywhere but in master and tablet server.  Otherwise,
corresponding group validator could strike while running binaries
that link the cfile library (e.g. the kudu CLI tool) at machines
with less than 256 MB of available memory.

Change-Id: I51f263890c91251f0d13c826c40dfd20fe284b59
---
M src/kudu/cfile/block_cache.cc
M src/kudu/consensus/CMakeLists.txt
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/master/master_main.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/tablet_server_main.cc
M src/kudu/util/flags.cc
M src/kudu/util/metrics.cc
M src/kudu/util/metrics.h
10 files changed, 50 insertions(+), 42 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/10342/8
--
To view, visit http://gerrit.cloudera.org:8080/10342
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I51f263890c91251f0d13c826c40dfd20fe284b59
Gerrit-Change-Number: 10342
Gerrit-PatchSet: 8
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] thirdparty: tweak clang compiler flags

[Dan Burkert (Code Review)]

Hello Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: thirdparty: tweak clang compiler flags
..

thirdparty: tweak clang compiler flags

Prior to this small tweak the thirdparty clang build output thousands of
warnings similar to this one when compiling with clang:

Building CXX object 
lib/Analysis/CMakeFiles/LLVMAnalysis.dir/LazyValueInfo.cpp.o
clang: warning: 
-Wl,-rpath,/Users/dan/src/cpp/kudu/thirdparty/installed/uninstrumented/lib: 
'linker' input unused [-Wunused-command-line-argument]
clang: warning: argument unused during compilation: 
'-L/Users/dan/src/cpp/kudu/thirdparty/installed/uninstrumented/lib' 
[-Wunused-command-line-argument]

Change-Id: Ide4ddbff14d3745c6f2c2f9b14b00da790a6cec6
---
M thirdparty/build-definitions.sh
1 file changed, 16 insertions(+), 13 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ide4ddbff14d3745c6f2c2f9b14b00da790a6cec6
Gerrit-Change-Number: 10352
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] thirdparty: tweak clang compiler flags

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10352 )

Change subject: thirdparty: tweak clang compiler flags
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10352/3/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

http://gerrit.cloudera.org:8080/#/c/10352/3/thirdparty/build-definitions.sh@278
PS3, Line 278:   # Depend on zlib from the thirdparty tree. It's an 
optional dependency for
> Can you add to this comment that we don't need to do this for TSAN because
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide4ddbff14d3745c6f2c2f9b14b00da790a6cec6
Gerrit-Change-Number: 10352
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 10 May 2018 16:59:50 +
Gerrit-HasComments: Yes

[kudu-CR] [flags] run validators after processing help flags

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10342 )

Change subject: [flags] run validators after processing help flags
..


Patch Set 7:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/cfile/block_cache.cc
File src/kudu/cfile/block_cache.cc:

http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/cfile/block_cache.cc@45
PS7, Line 45: // For details, see https://gerrit.cloudera.org/#/c/10342/
You can omit this; it'll be in the git commit message anyway.


http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/master/master_main.cc
File src/kudu/master/master_main.cc:

http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/master/master_main.cc@58
PS7, Line 58: to
omit


http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/master/master_main.cc@60
PS7, Line 60:   SetCommandLineOptionWithMode("force_block_cache_capacity", 
"false",
Check that the return value is not empty.


http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/tools/CMakeLists.txt
File src/kudu/tools/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/tools/CMakeLists.txt@18
PS7, Line 18: set(LINK_LIBS
This is only used in kudu_tools_util, so maybe just merge it in there?


http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/tools/CMakeLists.txt@61
PS7, Line 61:   server_process
Sort order.


http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/tserver/tablet_server_main.cc
File src/kudu/tserver/tablet_server_main.cc:

http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/tserver/tablet_server_main.cc@63
PS7, Line 63: to
omit


http://gerrit.cloudera.org:8080/#/c/10342/7/src/kudu/tserver/tablet_server_main.cc@65
PS7, Line 65:   SetCommandLineOptionWithMode("force_block_cache_capacity", 
"false",
Check return value.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I51f263890c91251f0d13c826c40dfd20fe284b59
Gerrit-Change-Number: 10342
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 10 May 2018 16:28:21 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2426 Fix WRONG SERVER UUID case in ksck

[Attila Bukor (Code Review)]

Attila Bukor has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10293 )

Change subject: KUDU-2426 Fix WRONG_SERVER_UUID case in ksck
..


Patch Set 4:

(1 comment)

> Patch Set 2:
>
> (1 comment)

thanks for the .

http://gerrit.cloudera.org:8080/#/c/10293/2/src/kudu/tools/ksck_remote.cc
File src/kudu/tools/ksck_remote.cc:

http://gerrit.cloudera.org:8080/#/c/10293/2/src/kudu/tools/ksck_remote.cc@165
PS2, Line 165: if (response_uuid != uuid()) {
> What do you think about changing the signature of FetchInfo() to:
thanks for the tip. as for the masters, it's a good idea, but I'd rather do it 
in a separate patch as currently there's no WRONG_SERVER_UUID for this at all 
so it feels weird to add it in this bugfix patch. what do you think?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b4f50fe4dd94450b4f2e34dbad315bd761b071f
Gerrit-Change-Number: 10293
Gerrit-PatchSet: 4
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 10 May 2018 12:48:13 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2426 Fix WRONG SERVER UUID case in ksck

[Attila Bukor (Code Review)]

Hello Will Berkeley, Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Todd 
Lipcon,

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

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

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

Change subject: KUDU-2426 Fix WRONG_SERVER_UUID case in ksck
..

KUDU-2426 Fix WRONG_SERVER_UUID case in ksck

RemoteKsckTabletServer::FetchInfo() returned a Status::RemoteError since
KUDU-2364 to indicate a UUID mismatch.
Ksck::FetchInfoFromTabletServers() checked for this Status, and this
resulted in any RemoteError showing as a WRONG_SERVER_UUID in the
tablet server health list.

Change-Id: I2b4f50fe4dd94450b4f2e34dbad315bd761b071f
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote-test.cc
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tools/ksck_results.cc
7 files changed, 27 insertions(+), 23 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2b4f50fe4dd94450b4f2e34dbad315bd761b071f
Gerrit-Change-Number: 10293
Gerrit-PatchSet: 4
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2426 Fix WRONG SERVER UUID case in ksck

[Attila Bukor (Code Review)]

Hello Will Berkeley, Kudu Jenkins, Andrew Wong, Adar Dembo, Todd Lipcon,

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

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

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

Change subject: KUDU-2426 Fix WRONG_SERVER_UUID case in ksck
..

KUDU-2426 Fix WRONG_SERVER_UUID case in ksck

RemoteKsckTabletServer::FetchInfo() returned a Status::RemoteError since
KUDU-2364 to indicate a UUID mismatch.
Ksck::FetchInfoFromTabletServers() checked for this Status, and this
resulted in any RemoteError showing as a WRONG_SERVER_UUID in the
tablet server health list.

Change-Id: I2b4f50fe4dd94450b4f2e34dbad315bd761b071f
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote-test.cc
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tools/ksck_results.cc
7 files changed, 27 insertions(+), 23 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2b4f50fe4dd94450b4f2e34dbad315bd761b071f
Gerrit-Change-Number: 10293
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 

[kudu-CR] KUDU-2287 Expose election failures as metrics

[Attila Bukor (Code Review)]

Hello Will Berkeley, Tidy Bot, Kudu Jenkins, Todd Lipcon,

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

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

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

Change subject: KUDU-2287 Expose election failures as metrics
..

KUDU-2287 Expose election failures as metrics

This patch exposes as a metric the number of election failures since the
last successful election attempt, as well as the time since there was a
stable leader for the tablet.

Change-Id: I1b25df258cdba7bdae7bb2d7b4eb3d73b53425c3
---
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
2 files changed, 46 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/10076/13
--
To view, visit http://gerrit.cloudera.org:8080/10076
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1b25df258cdba7bdae7bb2d7b4eb3d73b53425c3
Gerrit-Change-Number: 10076
Gerrit-PatchSet: 13
Gerrit-Owner: Attila Bukor 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley