[kudu-CR] env: add WriteV() API
Adar Dembo has posted comments on this change. Change subject: env: add WriteV() API .. Patch Set 4: Verified+1 Overriding Jenkins; the only failure appears to be in dist-test and is unlikely to be related to this patch. -- To view, visit http://gerrit.cloudera.org:8080/6800 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30acfa2e4918ef945c55646647913b36a07daaa4 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: No
[kudu-CR] env: add WriteV() API
Adar Dembo has submitted this change and it was merged. Change subject: env: add WriteV() API .. env: add WriteV() API Adds WriteV() methods to RWFile and WritableFile that allows writing multiple data Slices in one call. The implementation leverages the pwritev system call when possible and simulates it with pwrite calls when unavailable. Additionally adds WriteV()/AppendV() methods to the block manager abstraction. These methods will be used in KUDU-463 to support writing checksums and block data in a single call. Change-Id: I30acfa2e4918ef945c55646647913b36a07daaa4 Reviewed-on: http://gerrit.cloudera.org:8080/6800 Reviewed-by: Adar Dembo Tested-by: Adar Dembo --- M src/kudu/cfile/cfile_writer.cc M src/kudu/cfile/cfile_writer.h M src/kudu/consensus/log_util.cc M src/kudu/fs/block_manager.h M src/kudu/fs/file_block_manager.cc M src/kudu/fs/log_block_manager.cc M src/kudu/util/env-test.cc M src/kudu/util/env.h M src/kudu/util/env_posix.cc M src/kudu/util/file_cache.cc 10 files changed, 200 insertions(+), 133 deletions(-) Approvals: Adar Dembo: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/6800 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I30acfa2e4918ef945c55646647913b36a07daaa4 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Tidy Bot
[kudu-CR] env: add WriteV() API
Adar Dembo has posted comments on this change. Change subject: env: add WriteV() API .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/6800 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30acfa2e4918ef945c55646647913b36a07daaa4 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: No
[kudu-CR] env: add WriteV() API
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6800 to look at the new patch set (#4). Change subject: env: add WriteV() API .. env: add WriteV() API Adds WriteV() methods to RWFile and WritableFile that allows writing multiple data Slices in one call. The implementation leverages the pwritev system call when possible and simulates it with pwrite calls when unavailable. Additionally adds WriteV()/AppendV() methods to the block manager abstraction. These methods will be used in KUDU-463 to support writing checksums and block data in a single call. Change-Id: I30acfa2e4918ef945c55646647913b36a07daaa4 --- M src/kudu/cfile/cfile_writer.cc M src/kudu/cfile/cfile_writer.h M src/kudu/consensus/log_util.cc M src/kudu/fs/block_manager.h M src/kudu/fs/file_block_manager.cc M src/kudu/fs/log_block_manager.cc M src/kudu/util/env-test.cc M src/kudu/util/env.h M src/kudu/util/env_posix.cc M src/kudu/util/file_cache.cc 10 files changed, 200 insertions(+), 133 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/00/6800/4 -- To view, visit http://gerrit.cloudera.org:8080/6800 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I30acfa2e4918ef945c55646647913b36a07daaa4 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] env: add WriteV() API
Adar Dembo has posted comments on this change. Change subject: env: add WriteV() API .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/6800/3/src/kudu/fs/block_manager.h File src/kudu/fs/block_manager.h: Line 120: virtual Status AppendV(const vector& data) = 0; This is in a header file, so you should keep the std:: prefix. -- To view, visit http://gerrit.cloudera.org:8080/6800 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30acfa2e4918ef945c55646647913b36a07daaa4 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
[kudu-CR] env: add WriteV() API
Grant Henke has posted comments on this change. Change subject: env: add WriteV() API .. Patch Set 1: (9 comments) http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/cfile/cfile_writer.cc File src/kudu/cfile/cfile_writer.cc: Line 478: } > Any particular reason to preserve this? It's a private function; if it's no Done Line 480: Status CFileWriter::WriteRawDataV(const vector &data) { > vector& Done http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/cfile/cfile_writer.h File src/kudu/cfile/cfile_writer.h: Line 189: Status WriteRawDataV(const vector &data); > Should be vector& (& next to the type). Done http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: Line 1063: virtual Status AppendV(const std::vector& data) OVERRIDE; > Don't need std:: prefix here. Done http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/util/env-test.cc File src/kudu/util/env-test.cc: Line 163: // Force short reads to half the slice length > You mean writes? Done http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/util/env.h File src/kudu/util/env.h: Line 448: virtual Status AppendV(const std::vector &data) = 0; > std::vector& Done Line 543: virtual Status WriteV(uint64_t offset, const vector &data) = 0; > Need std:: prefix here. Also vector& Done http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/util/env_posix.cc File src/kudu/util/env_posix.cc: Line 408: // Convert the results into the iovec vector to request > Nit: terminate comments that are English sentences with a period. Done Line 571: Status s = DoWriteV(fd_, filename_, filesize_, data); > Hmm, this seems weird; shouldn't we RETURN_NOT_OK() and not touch filesize_ Done -- To view, visit http://gerrit.cloudera.org:8080/6800 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30acfa2e4918ef945c55646647913b36a07daaa4 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
[kudu-CR] env: add WriteV() API
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6800 to look at the new patch set (#3). Change subject: env: add WriteV() API .. env: add WriteV() API Adds WriteV() methods to RWFile and WritableFile that allows writing multiple data Slices in one call. The implementation leverages the pwritev system call when possible and simulates it with pwrite calls when unavailable. Additionally adds WriteV()/AppendV() methods to the block manager abstraction. These methods will be used in KUDU-463 to support writing checksums and block data in a single call. Change-Id: I30acfa2e4918ef945c55646647913b36a07daaa4 --- M src/kudu/cfile/cfile_writer.cc M src/kudu/cfile/cfile_writer.h M src/kudu/consensus/log_util.cc M src/kudu/fs/block_manager.h M src/kudu/fs/file_block_manager.cc M src/kudu/fs/log_block_manager.cc M src/kudu/util/env-test.cc M src/kudu/util/env.h M src/kudu/util/env_posix.cc M src/kudu/util/file_cache.cc 10 files changed, 200 insertions(+), 133 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/00/6800/3 -- To view, visit http://gerrit.cloudera.org:8080/6800 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I30acfa2e4918ef945c55646647913b36a07daaa4 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] env: add WriteV() API
Adar Dembo has posted comments on this change. Change subject: env: add WriteV() API .. Patch Set 1: (10 comments) http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/cfile/cfile_writer.cc File src/kudu/cfile/cfile_writer.cc: PS1, Line 476: Status CFileWriter::WriteRawData(const Slice& data) { : return WriteRawDataV({ data }); : } Any particular reason to preserve this? It's a private function; if it's not being used anymore, let's remove it. PS1, Line 480: vector & vector& http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/cfile/cfile_writer.h File src/kudu/cfile/cfile_writer.h: PS1, Line 189: vector & Should be vector& (& next to the type). http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/fs/file_block_manager.cc File src/kudu/fs/file_block_manager.cc: PS1, Line 229: std:: Don't need. http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: PS1, Line 1063: std:: Don't need std:: prefix here. http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/util/env-test.cc File src/kudu/util/env-test.cc: PS1, Line 163: reads You mean writes? http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/util/env.h File src/kudu/util/env.h: PS1, Line 448: & std::vector& PS1, Line 543: vector Need std:: prefix here. Also vector& http://gerrit.cloudera.org:8080/#/c/6800/1/src/kudu/util/env_posix.cc File src/kudu/util/env_posix.cc: Line 408: // Convert the results into the iovec vector to request Nit: terminate comments that are English sentences with a period. Line 571: Status s = DoWriteV(fd_, filename_, filesize_, data); Hmm, this seems weird; shouldn't we RETURN_NOT_OK() and not touch filesize_ or pending_sync_ on failure? -- To view, visit http://gerrit.cloudera.org:8080/6800 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30acfa2e4918ef945c55646647913b36a07daaa4 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
[kudu-CR] env: add WriteV() API
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6800 to look at the new patch set (#2). Change subject: env: add WriteV() API .. env: add WriteV() API Adds WriteV() methods to RWFile and WritableFile that allows writing multiple data Slices in one call. The implementation leverages the pwritev system call when possible and simulates it with pwrite calls when unavailable. Additionally adds WriteV()/AppendV() methods to the block manager abstraction. These methods will be used in KUDU-463 to support writing checksums and block data in a single call. Change-Id: I30acfa2e4918ef945c55646647913b36a07daaa4 --- M src/kudu/cfile/cfile_writer.cc M src/kudu/cfile/cfile_writer.h M src/kudu/consensus/log_util.cc M src/kudu/fs/block_manager.h M src/kudu/fs/file_block_manager.cc M src/kudu/fs/log_block_manager.cc M src/kudu/util/env-test.cc M src/kudu/util/env.h M src/kudu/util/env_posix.cc M src/kudu/util/file_cache.cc 10 files changed, 202 insertions(+), 130 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/00/6800/2 -- To view, visit http://gerrit.cloudera.org:8080/6800 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I30acfa2e4918ef945c55646647913b36a07daaa4 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] env: add WriteV() API
Grant Henke has uploaded a new change for review. http://gerrit.cloudera.org:8080/6800 Change subject: env: add WriteV() API .. env: add WriteV() API Adds WriteV() methods to RWFile and WritableFile that allows writing multiple data Slices in one call. The implementation leverages the pwritev system call when possible and simulates it with pwrite calls when unavailable. Additionally adds WriteV()/AppendV() methods to the block manager abstraction. These methods will be used in KUDU-463 to support writing checksums and block data in a single call. Change-Id: I30acfa2e4918ef945c55646647913b36a07daaa4 --- M src/kudu/cfile/cfile_writer.cc M src/kudu/cfile/cfile_writer.h M src/kudu/consensus/log_util.cc M src/kudu/fs/block_manager.h M src/kudu/fs/file_block_manager.cc M src/kudu/fs/log_block_manager.cc M src/kudu/util/env-test.cc M src/kudu/util/env.h M src/kudu/util/env_posix.cc M src/kudu/util/file_cache.cc 10 files changed, 202 insertions(+), 130 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/00/6800/1 -- To view, visit http://gerrit.cloudera.org:8080/6800 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I30acfa2e4918ef945c55646647913b36a07daaa4 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke