[Tarantool-patches] [PATCH v2] fio/coio: Handle partial writes
Cyrill Gorcunov
gorcunov at gmail.com
Wed Dec 4 14:17:49 MSK 2019
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 <gorcunov at gmail.com>
---
branch gorcunov/gh-4651-partial-write-2
v2:
- Use write_should_retry helper to not duplicate code
- Return -1 on any other error, so that fio.write() return
false
src/lib/core/coio_file.c | 80 ++++++++++++++++++++++++++++++++++------
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, 166 insertions(+), 11 deletions(-)
diff --git a/src/lib/core/coio_file.c b/src/lib/core/coio_file.c
index 62388344e..af5fb30ce 100644
--- a/src/lib/core/coio_file.c
+++ b/src/lib/core/coio_file.c
@@ -161,13 +161,49 @@ coio_file_close(int fd)
return coio_wait_done(req, &eio);
}
+static inline int
+write_should_retry(void)
+{
+ /* Retry on O_NONBLOCK */
+ if (errno == EAGAIN)
+ return true;
+ /* Retry if been interrupted */
+ if (errno == EINTR)
+ return true;
+ /* Unexpected error */
+ return false;
+}
+
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 (!write_should_retry()) {
+ pos = -1;
+ break;
+ }
+ } else {
+ left -= res;
+ pos += res;
+ }
+ }
+ return pos;
}
ssize_t
@@ -200,7 +236,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 +249,30 @@ 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 (!write_should_retry()) {
+ pos = -1;
+ 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
More information about the Tarantool-patches
mailing list