Adar Dembo 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 2:

(2 comments)

Besides Todd's suggestion of running a Zipfian update YCSB workload, could you 
also run TestMemRowSet.TestMemRowSetInsertCountAndScan before and after the 
change (with sufficiently high values of --roundtrip_num_rows and 
--num_scan_passes)?

We don't have a microbenchmark in memrowset-test that simulates your use case 
(zipfian distribution of updates); could you add one and use it to quantify the 
performance improvement of this patch?

http://gerrit.cloudera.org:8080/#/c/13412/2/src/kudu/tablet/memrowset.h
File src/kudu/tablet/memrowset.h:

http://gerrit.cloudera.org:8080/#/c/13412/2/src/kudu/tablet/memrowset.h@102
PS2, Line 102:       // Pointer to the first mutation which has been applied to 
this row. Each
             :       // mutation is an instance of the Mutation class, making 
up a singly-linked
             :       // list for any mutations applied to the row.
             :       Mutation* redo_head;
             :
             :       // Pointer to the last mutation which has been applied to 
this row. Add this
             :       // member for appending mutation in list more efficient.
             :       Mutation* redo_tail;
For added clarity, I'd rewrite this:

  // Pointers to the first and last mutations that have been applied to this 
row.
  // Together they comprise a doubly-linked list of all of the row's mutations,
  // with the head and tail used for efficient iteration and insertion 
respectively.
  Mutation* redo_head;
  Mutation* redo_tail;


http://gerrit.cloudera.org:8080/#/c/13412/2/src/kudu/tablet/mutation.h
File src/kudu/tablet/mutation.h:

http://gerrit.cloudera.org:8080/#/c/13412/2/src/kudu/tablet/mutation.h@87
PS2, Line 87:   void AppendToListAtomic(MRSRow::Header *list);
Rather than passing this MRS-specific data structure in, can we pass the head 
and tail of the list as separate arguments?



--
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: 2
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: Fri, 24 May 2019 07:37:17 +0000
Gerrit-HasComments: Yes

Reply via email to