ZhangYao has posted comments on this change. (
http://gerrit.cloudera.org:8080/13412 )
Change subject: KUDU-2826: Add tail for mutation list to accelerate update in
memrowset
......................................................................
Patch Set 4:
(4 comments)
@Adar Dembo
I rerun the TestMemRowSet.TestMemRowSetInsertCountAndScan with
--roundtrip_num_rows = 10000000, here is the result.
[Before change]:
Time spent Inserting rows: real 7.259s user 6.711s sys 0.505s
Time spent Counting rows: real 0.239s user 0.145s sys 0.092s
Time spent Scanning rows where none are committed: real 0.196s user 0.195s sys
0.000s
Time spent Scanning rows where all are committed: real 0.491s user 0.488s sys
0.000s
[After change]:
Time spent Inserting rows: real 7.340s user 6.690s sys 0.603s
Time spent Counting rows: real 0.247s user 0.145s sys 0.101s
Time spent Scanning rows where none are committed: real 0.204s user 0.203s sys
0.000s
Time spent Scanning rows where all are committed: real 0.401s user 0.398s sys
0.000s
The result illustrates this change does not affect the insert and scan.
I also rerun TestMemRowSetUpdatePerformance with --update_ratio = 0.2 and
--update_ratio = 5000.
[Before change]:
Time spent Updating rows: real 156.542s user 155.164s sys 0.304s
[After change]:
Time spent Updating rows: real 3.140s user 3.112s sys 0.007s
The result illustrates this change optimize the updates performance.
Finally, we do YCSB here for a zipfian update workload before and after change.
Workload:
recordcount=1000
operationcount=1000000
workload=com.yahoo.ycsb.workloads.CoreWorkload
readallfields=true
readproportion=0.2
updateproportion=0.8
scanproportion=0
insertproportion=0
requestdistribution=zipfian
The performance before:
Update success: 800304,
AverageLatency(us): 93.81846898178692,
MinLatency(us): 1,
MaxLatency(us): 837631,
95thPercentileLatency(us): 6
99thPercentileLatency(us):12
The performance after:
Update success: 800153,
AverageLatency(us): 4.785403541572674,
MinLatency(us): 1,
MaxLatency(us): 92287,
95thPercentileLatency(us): 6
99thPercentileLatency(us): 11
http://gerrit.cloudera.org:8080/#/c/13412/3/src/kudu/tablet/memrowset.h
File src/kudu/tablet/memrowset.h:
http://gerrit.cloudera.org:8080/#/c/13412/3/src/kudu/tablet/memrowset.h@175
PS3, Line 175: Header *header_;
> Hmm, I misled you a bit: this isn't actually a doubly-linked list, because
Got that.
http://gerrit.cloudera.org:8080/#/c/13412/3/src/kudu/tablet/memrowset.cc
File src/kudu/tablet/memrowset.cc:
http://gerrit.cloudera.org:8080/#/c/13412/3/src/kudu/tablet/memrowset.cc@77
PS3, Line 77: mut_tail->changelist().ToString(*schema()),
: s.ToString());
> nit: this may be slightly more readable if written with Substitute().
Done
http://gerrit.cloudera.org:8080/#/c/13412/3/src/kudu/tablet/memrowset.cc@80
PS3, Line 80: return decoder.is_delete();
: }
:
: namespace {
> nit: could simplify this to be `return decoder.is_delete()`.
Done
http://gerrit.cloudera.org:8080/#/c/13412/3/src/kudu/tablet/mutation.h
File src/kudu/tablet/mutation.h:
http://gerrit.cloudera.org:8080/#/c/13412/3/src/kudu/tablet/mutation.h@86
PS3, Line 86: void AppendToListAtomic(Mutation** redo_head, Mutation**
redo_tail);
> Nit: while you're here, could you change this "Mutation**" instead of "Muta
Done
--
To view, visit http://gerrit.cloudera.org:8080/13412
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I945f332f8241ecb3cc6ab66e67ee3ee2b4e49be8
Gerrit-Change-Number: 13412
Gerrit-PatchSet: 4
Gerrit-Owner: ZhangYao <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Yao Xu <[email protected]>
Gerrit-Reviewer: ZhangYao <[email protected]>
Gerrit-Comment-Date: Tue, 28 May 2019 12:56:49 +0000
Gerrit-HasComments: Yes
