From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-f49.google.com (mail-lf1-f49.google.com [209.85.167.49]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id ABBCA46970F for ; Tue, 26 Nov 2019 21:05:13 +0300 (MSK) Received: by mail-lf1-f49.google.com with SMTP id d6so14895487lfc.0 for ; Tue, 26 Nov 2019 10:05:13 -0800 (PST) From: Cyrill Gorcunov Date: Tue, 26 Nov 2019 21:05:03 +0300 Message-Id: <20191126180503.22238-1-gorcunov@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH] fio/coio: Handle partial writes List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tml Writting less bytes than requested is perfectly fine from OS point of view. In turn our fio.write/pwrite api simply returns 'true' even if only a part of a buffer has been written. We can't change the api without breaking backward compatibility (otherwise we could simply return number of bytes written, instead of current true/false sign). For this sake both coio_write and coio_pwrite tries to deliver complete data in a cycle. Fixes #4651 Signed-off-by: Cyrill Gorcunov --- https://github.com/tarantool/tarantool/compare/gorcunov/gh-4651-partial-write https://github.com/tarantool/tarantool/issues/4651 src/lib/core/coio_file.c | 63 ++++++++++++++++++++++++++++------ src/lib/core/errinj.h | 1 + test/app/fio.result | 73 ++++++++++++++++++++++++++++++++++++++++ test/app/fio.test.lua | 21 ++++++++++++ test/box/errinj.result | 2 ++ 5 files changed, 149 insertions(+), 11 deletions(-) diff --git a/src/lib/core/coio_file.c b/src/lib/core/coio_file.c index 62388344e..90903c8df 100644 --- a/src/lib/core/coio_file.c +++ b/src/lib/core/coio_file.c @@ -164,10 +164,31 @@ coio_file_close(int fd) ssize_t coio_pwrite(int fd, const void *buf, size_t count, off_t offset) { - INIT_COEIO_FILE(eio); - eio_req *req = eio_write(fd, (void *) buf, count, offset, - 0, coio_complete, &eio); - return coio_wait_done(req, &eio); + struct errinj *inj = errinj(ERRINJ_COIO_WRITE_CHUNK, ERRINJ_INT); + ssize_t left = count, pos = 0, res, chunk; + eio_req *req; + + while (left > 0) { + INIT_COEIO_FILE(eio); + + if (inj != NULL && inj->iparam > 0) + chunk = (ssize_t)inj->iparam; + else + chunk = left; + + req = eio_write(fd, (void *)buf + pos, chunk, + offset + pos, EIO_PRI_DEFAULT, + coio_complete, &eio); + res = coio_wait_done(req, &eio); + if (res < 0) { + if (errno != EINTR) + break; + } else { + left -= res; + pos += res; + } + } + return pos; } ssize_t @@ -200,7 +221,12 @@ coio_preadn(int fd, void *buf, size_t count, off_t offset) static void coio_do_write(eio_req *req) { + struct errinj *inj = errinj(ERRINJ_COIO_WRITE_CHUNK, ERRINJ_INT); struct coio_file_task *eio = (struct coio_file_task *)req->data; + + if (inj != NULL && inj->iparam > 0) + eio->write.count = (ssize_t)inj->iparam; + req->result = write(eio->write.fd, eio->write.buf, eio->write.count); eio->errorno = errno; } @@ -208,13 +234,28 @@ coio_do_write(eio_req *req) ssize_t coio_write(int fd, const void *buf, size_t count) { - INIT_COEIO_FILE(eio); - eio.write.buf = buf; - eio.write.count = count; - eio.write.fd = fd; - eio_req *req = eio_custom(coio_do_write, 0, - coio_complete, &eio); - return coio_wait_done(req, &eio); + ssize_t left = count, pos = 0, res; + eio_req *req; + + while (left > 0) { + INIT_COEIO_FILE(eio); + + eio.write.buf = buf + pos; + eio.write.count = left; + eio.write.fd = fd; + + req = eio_custom(coio_do_write, EIO_PRI_DEFAULT, + coio_complete, &eio); + res = coio_wait_done(req, &eio); + if (res < 0) { + if (errno != EINTR) + break; + } else { + left -= res; + pos += res; + } + } + return pos; } static void diff --git a/src/lib/core/errinj.h b/src/lib/core/errinj.h index 3072a00ea..f90554d06 100644 --- a/src/lib/core/errinj.h +++ b/src/lib/core/errinj.h @@ -134,6 +134,7 @@ struct errinj { _(ERRINJ_SQL_NAME_NORMALIZATION, ERRINJ_BOOL, {.bparam = false}) \ _(ERRINJ_COIO_SENDFILE_CHUNK, ERRINJ_INT, {.iparam = -1}) \ _(ERRINJ_SWIM_FD_ONLY, ERRINJ_BOOL, {.bparam = false}) \ + _(ERRINJ_COIO_WRITE_CHUNK, ERRINJ_INT, {.iparam = -1}) \ ENUM0(errinj_id, ERRINJ_LIST); extern struct errinj errinjs[]; diff --git a/test/app/fio.result b/test/app/fio.result index f83c43f44..6eafa24e6 100644 --- a/test/app/fio.result +++ b/test/app/fio.result @@ -776,6 +776,12 @@ file5 = fio.pathjoin(tree, 'file.5') file6 = fio.pathjoin(tree, 'file.6') --- ... +file7 = fio.pathjoin(tree, 'file.7') +--- +... +file8 = fio.pathjoin(tree, 'file.8') +--- +... fh1 = fio.open(file1, { 'O_RDWR', 'O_TRUNC', 'O_CREAT' }, tonumber('0777', 8)) --- ... @@ -865,6 +871,73 @@ fh6:close() --- - true ... +-- test partial write/pwrite via one byte transfer +fh7 = fio.open(file7, { 'O_RDWR', 'O_TRUNC', 'O_CREAT' }, tonumber('0644', 8)) +--- +... +errinj.set('ERRINJ_COIO_WRITE_CHUNK', 1) +--- +- ok +... +fh7:write("one byte transfer, write") +--- +- true +... +errinj.set('ERRINJ_COIO_WRITE_CHUNK', -1) +--- +- ok +... +fh7:seek(0) +--- +- 0 +... +fh7:read() +--- +- one byte transfer, write +... +fh7:close() +--- +- true +... +fh8 = fio.open(file8, { 'O_RDWR', 'O_TRUNC', 'O_CREAT' }, tonumber('0644', 8)) +--- +... +errinj.set('ERRINJ_COIO_WRITE_CHUNK', 1) +--- +- ok +... +fh8:pwrite("one byte transfer, ", 0) +--- +- true +... +fh8:seek(0) +--- +- 0 +... +fh8:read() +--- +- 'one byte transfer, ' +... +fh8:pwrite("pwrite", 19) +--- +- true +... +errinj.set('ERRINJ_COIO_WRITE_CHUNK', -1) +--- +- ok +... +fh8:seek(0) +--- +- 0 +... +fh8:read() +--- +- one byte transfer, pwrite +... +fh8:close() +--- +- true +... res, err = fio.copyfile(fio.pathjoin(tmp1, 'not_exists.txt'), tmp1) --- ... diff --git a/test/app/fio.test.lua b/test/app/fio.test.lua index 56c957d8a..3ed06bcda 100644 --- a/test/app/fio.test.lua +++ b/test/app/fio.test.lua @@ -251,6 +251,8 @@ file3 = fio.pathjoin(tree, 'file.3') file4 = fio.pathjoin(tree, 'file.4') file5 = fio.pathjoin(tree, 'file.5') file6 = fio.pathjoin(tree, 'file.6') +file7 = fio.pathjoin(tree, 'file.7') +file8 = fio.pathjoin(tree, 'file.8') fh1 = fio.open(file1, { 'O_RDWR', 'O_TRUNC', 'O_CREAT' }, tonumber('0777', 8)) fh1:write("gogo") @@ -280,6 +282,25 @@ fh6:seek(0) fh6:read() fh6:close() +-- test partial write/pwrite via one byte transfer +fh7 = fio.open(file7, { 'O_RDWR', 'O_TRUNC', 'O_CREAT' }, tonumber('0644', 8)) +errinj.set('ERRINJ_COIO_WRITE_CHUNK', 1) +fh7:write("one byte transfer, write") +errinj.set('ERRINJ_COIO_WRITE_CHUNK', -1) +fh7:seek(0) +fh7:read() +fh7:close() +fh8 = fio.open(file8, { 'O_RDWR', 'O_TRUNC', 'O_CREAT' }, tonumber('0644', 8)) +errinj.set('ERRINJ_COIO_WRITE_CHUNK', 1) +fh8:pwrite("one byte transfer, ", 0) +fh8:seek(0) +fh8:read() +fh8:pwrite("pwrite", 19) +errinj.set('ERRINJ_COIO_WRITE_CHUNK', -1) +fh8:seek(0) +fh8:read() +fh8:close() + res, err = fio.copyfile(fio.pathjoin(tmp1, 'not_exists.txt'), tmp1) res err:match("failed to copy") ~= nil diff --git a/test/box/errinj.result b/test/box/errinj.result index a148346e8..9b831769f 100644 --- a/test/box/errinj.result +++ b/test/box/errinj.result @@ -25,6 +25,8 @@ errinj.info() state: 0 ERRINJ_WAL_WRITE: state: false + ERRINJ_COIO_WRITE_CHUNK: + state: -1 ERRINJ_HTTPC_EXECUTE: state: false ERRINJ_VYRUN_DATA_READ: -- 2.20.1