[kudu-CR] Update debug partition and row printing

[Dan Burkert (Code Review)]

Dan Burkert has submitted this change and it was merged.

Change subject: Update debug partition and row printing
..


Update debug partition and row printing

This commit updates the debug printing of rows and partitions in
metrics, log messages, and web UIs. The new format is one we have
settled on after consulting with committers on Apache Impala. In all
cases I think it makes it easier to work with complex partitions. The
most invasive aspect of this change is that string and binary column
values are now printed with quotes, which affects a bunch of otherwise
unrelated tests.

I attempted to minimize as much of the duplication as possible in debug
formatting functions in partition.cc, but there is still a lot of
duplication that was unavoidable (this was true before making this
change as well).

Change-Id: I4c444b155fe6621af65b86020be105fe56ae18ef
Reviewed-on: http://gerrit.cloudera.org:8080/5262
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/client/batcher.cc
M src/kudu/client/client-test.cc
M src/kudu/common/encoded_key-test.cc
M src/kudu/common/key_util-test.cc
M src/kudu/common/partial_row-test.cc
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/row_operations-test.cc
M src/kudu/common/scan_spec-test.cc
M src/kudu/common/schema-test.cc
M src/kudu/common/types.h
M src/kudu/master/master-path-handlers.cc
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/composite-pushdown-test.cc
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tools/kudu-ts-cli-test.cc
M src/kudu/tserver/tablet_server-test.cc
25 files changed, 507 insertions(+), 472 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I4c444b155fe6621af65b86020be105fe56ae18ef
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Update debug partition and row printing

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Update debug partition and row printing
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4c444b155fe6621af65b86020be105fe56ae18ef
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Update debug partition and row printing

[Dan Burkert (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Update debug partition and row printing
..

Update debug partition and row printing

This commit updates the debug printing of rows and partitions in
metrics, log messages, and web UIs. The new format is one we have
settled on after consulting with committers on Apache Impala. In all
cases I think it makes it easier to work with complex partitions. The
most invasive aspect of this change is that string and binary column
values are now printed with quotes, which affects a bunch of otherwise
unrelated tests.

I attempted to minimize as much of the duplication as possible in debug
formatting functions in partition.cc, but there is still a lot of
duplication that was unavoidable (this was true before making this
change as well).

Change-Id: I4c444b155fe6621af65b86020be105fe56ae18ef
---
M src/kudu/client/batcher.cc
M src/kudu/client/client-test.cc
M src/kudu/common/encoded_key-test.cc
M src/kudu/common/key_util-test.cc
M src/kudu/common/partial_row-test.cc
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/row_operations-test.cc
M src/kudu/common/scan_spec-test.cc
M src/kudu/common/schema-test.cc
M src/kudu/common/types.h
M src/kudu/master/master-path-handlers.cc
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/composite-pushdown-test.cc
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tools/kudu-ts-cli-test.cc
M src/kudu/tserver/tablet_server-test.cc
25 files changed, 507 insertions(+), 472 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c444b155fe6621af65b86020be105fe56ae18ef
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Update debug partition and row printing

[Dan Burkert (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Update debug partition and row printing
..

Update debug partition and row printing

This commit updates the debug printing of rows and partitions in
metrics, log messages, and web UIs. The new format is one we have
settled on after consulting with committers on Apache Impala. In all
cases I think it makes it easier to work with complex partitions. The
most invasive aspect of this change is that string and binary column
values are now printed with quotes, which affects a bunch of otherwise
unrelated tests.

I attempted to minimize as much of the duplication as possible in debug
formatting functions in partition.cc, but there is still a lot of
duplication that was unavoidable (this was true before making this
change as well).

Change-Id: I4c444b155fe6621af65b86020be105fe56ae18ef
---
M src/kudu/client/batcher.cc
M src/kudu/client/client-test.cc
M src/kudu/common/encoded_key-test.cc
M src/kudu/common/key_util-test.cc
M src/kudu/common/partial_row-test.cc
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/row_operations-test.cc
M src/kudu/common/scan_spec-test.cc
M src/kudu/common/schema-test.cc
M src/kudu/common/types.h
M src/kudu/master/master-path-handlers.cc
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/composite-pushdown-test.cc
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tools/kudu-ts-cli-test.cc
M src/kudu/tserver/tablet_server-test.cc
25 files changed, 505 insertions(+), 470 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c444b155fe6621af65b86020be105fe56ae18ef
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Update debug partition and row printing

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Update debug partition and row printing
..


Patch Set 1:

(6 comments)

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

PS1, Line 585:   if (partition.hash_buckets_.size() != 
hash_bucket_schemas_.size()) {
 : return "";
 :   }
should this be a CHECK? would be indicative of programmer error, right? or 
perhaps a LOG(DFATAL) at least so we'd catch it in test builds?


Line 621:   components.emplace_back(Substitute("", 
s.ToString()));
same


Line 652: return "";
same


PS1, Line 689: oakc
not following this reference


Line 706: return "";
same


Line 810: ColumnIdsToColumnNames(schema, 
hash_bucket_schema.column_ids));
for these HTML output functions, we should be escaping the user-provided column 
names


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4c444b155fe6621af65b86020be105fe56ae18ef
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Update debug partition and row printing

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: Update debug partition and row printing
..


Patch Set 1:

screenshots of updated web UIs: http://imgur.com/a/VNlMA

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

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

[kudu-CR] Update debug partition and row printing

[Dan Burkert (Code Review)]

Hello Adar Dembo, Dimitris Tsirogiannis, Todd Lipcon,

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

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

to review the following change.

Change subject: Update debug partition and row printing
..

Update debug partition and row printing

This commit updates the debug printing of rows and partitions in
metrics, log messages, and web UIs. The new format is one we have
settled on after consulting with committers on Apache Impala. In all
cases I think it makes it easier to work with complex partitions. The
most invasive aspect of this change is that string and binary column
values are now printed with quotes, which affects a bunch of otherwise
unrelated tests.

I attempted to minimize as much of the duplication as possible in debug
formatting functions in partition.cc, but there is still a lot of
duplication that was unavoidable (this was true before making this
change as well).

Change-Id: I4c444b155fe6621af65b86020be105fe56ae18ef
---
M src/kudu/client/batcher.cc
M src/kudu/client/client-test.cc
M src/kudu/common/encoded_key-test.cc
M src/kudu/common/key_util-test.cc
M src/kudu/common/partial_row-test.cc
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/row_operations-test.cc
M src/kudu/common/scan_spec-test.cc
M src/kudu/common/schema-test.cc
M src/kudu/common/types.h
M src/kudu/master/master-path-handlers.cc
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/composite-pushdown-test.cc
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tools/kudu-ts-cli-test.cc
M src/kudu/tserver/tablet_server-test.cc
25 files changed, 479 insertions(+), 449 deletions(-)


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

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