[kudu-CR] KUDU-2844 (3/3): avoid copying plain/dict strings to RowBlock Arena

["Todd Lipcon (Code Review)" <ge...@cloudera.org>]

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

Change subject: KUDU-2844 (3/3): avoid copying plain/dict strings to RowBlock Arena
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/15802/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15802/3//COMMIT_MSG@42
PS3, Line 42: I waited until the data had been fully flushed from MRS before running
> Any chance compactions could have occurred and interfered with the block la
I think I just watched that the maintenance manager thread wasn't running. I'll run these benchmarks again just to be sure since it's been a while.


http://gerrit.cloudera.org:8080/#/c/15802/3/src/kudu/common/rowblock_memory.h
File src/kudu/common/rowblock_memory.h:

http://gerrit.cloudera.org:8080/#/c/15802/3/src/kudu/common/rowblock_memory.h@46
PS3, Line 46: template<class T>
            :   void RetainReference(T* item) {
> We're both storing and passing RowBlockRefCounted*s, but the callsites migh
this code changed a bit with the switch to shared_ptr, take a look and see what you think


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

http://gerrit.cloudera.org:8080/#/c/15802/3/src/kudu/integration-tests/full_stack-insert-scan-test.cc@204
PS3, Line 204:     // Enable fault tolerance.
> nit: would be nice to mention what's so special about fault tolerant scans.
just the different code path coverage


http://gerrit.cloudera.org:8080/#/c/15802/3/src/kudu/integration-tests/full_stack-insert-scan-test.cc@414
PS3, Line 414:                                              const string& msg,
> warning: parameter 'msg' is unused [misc-unused-parameters]
not sure why it thinks it's unused, I guess something weird about the LOG_TIMING macro and the clang analyzer



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93fa1f9fd401814a42dc5a1f3fd2ffb1286ac441
Gerrit-Change-Number: 15802
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Volodymyr Verovkin <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Aug 2020 22:02:12 +0000
Gerrit-HasComments: Yes