You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2016/08/12 00:56:16 UTC

[kudu-CR] KUDU-1555. PBC Flush() method should be async

Hello Mike Percy, Adar Dembo,

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

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

to review the following change.

Change subject: KUDU-1555. PBC Flush() method should be async
......................................................................

KUDU-1555. PBC Flush() method should be async

WritablePBContainerFile::Flush is meant to flush the data
asynchronously. But, e6052ac accidentally regressed this functionality
and switched it to synchronous.

This caused a lack of parallelism in the LogBlockManager flush path,
since it uses PBC files for the metadata. The async flushing of LBM data
blocks ended up being basically synchronous, so each disk would have to
wait for the prior disk to complete its flush before it started any IO.

I tested this change on a YCSB workload and throughput increased almost
2x.

Change-Id: I721707070fe47e3377d791c95214f007c90d2263
---
M src/kudu/util/pb_util.cc
1 file changed, 1 insertion(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I721707070fe47e3377d791c95214f007c90d2263
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] KUDU-1555. PBC Flush() method should be async

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1555. PBC Flush() method should be async
......................................................................


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/2825/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I721707070fe47e3377d791c95214f007c90d2263
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No

[kudu-CR] KUDU-1555. PBC Flush() method should be async

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.

Change subject: KUDU-1555. PBC Flush() method should be async
......................................................................


Patch Set 2:

Wow, wish we had noticed this earlier

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I721707070fe47e3377d791c95214f007c90d2263
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No

[kudu-CR] KUDU-1555. PBC Flush() method should be async

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged.

Change subject: KUDU-1555. PBC Flush() method should be async
......................................................................


KUDU-1555. PBC Flush() method should be async

WritablePBContainerFile::Flush is meant to flush the data
asynchronously. But, e6052ac accidentally regressed this functionality
and switched it to synchronous.

This caused a lack of parallelism in the LogBlockManager flush path,
since it uses PBC files for the metadata. The async flushing of LBM data
blocks ended up being basically synchronous, so each disk would have to
wait for the prior disk to complete its flush before it started any IO.

I tested this change on a YCSB workload and throughput increased almost
2x.

Change-Id: I721707070fe47e3377d791c95214f007c90d2263
Reviewed-on: http://gerrit.cloudera.org:8080/3951
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>
---
M src/kudu/util/pb_util.cc
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Mike Percy: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I721707070fe47e3377d791c95214f007c90d2263
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] KUDU-1555. PBC Flush() method should be async

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1555. PBC Flush() method should be async
......................................................................


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/2824/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I721707070fe47e3377d791c95214f007c90d2263
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No

[kudu-CR] KUDU-1555. PBC Flush() method should be async

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.

Change subject: KUDU-1555. PBC Flush() method should be async
......................................................................


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I721707070fe47e3377d791c95214f007c90d2263
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No