Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH] fio/coio: Handle partial writes
@ 2019-11-26 18:05 Cyrill Gorcunov
  2019-12-04  9:44 ` Cyrill Gorcunov
  0 siblings, 1 reply; 14+ messages in thread
From: Cyrill Gorcunov @ 2019-11-26 18:05 UTC (permalink / raw)
  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 <gorcunov@gmail.com>
---
 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

^ permalink raw reply	[flat|nested] 14+ messages in thread
* [Tarantool-patches] [PATCH] fio/coio: handle partial writes
@ 2020-04-21 21:39 Cyrill Gorcunov
  2020-05-06 17:08 ` Alexander Turenko
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Cyrill Gorcunov @ 2020-04-21 21:39 UTC (permalink / raw)
  To: tml

Writting less bytes than requested is perfectly fine.
In turn our fio.write/pwrite api simply returns 'true'
even if only some part of a buffer has been written.

Thus make coio_write and coio_pwrite to write the
whole data in a cycle. Note in most situations there
will be only one pass, partial writes are really the
rare cases.

Fixes #4651

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
issue https://github.com/tarantool/tarantool/issues/4651
branch gorcunov/gh-4651-partial-write

 src/lib/core/coio_file.c | 62 ++++++++++++++++++++++++++++------
 src/lib/core/errinj.h    |  1 +
 test/app/fio.result      | 73 ++++++++++++++++++++++++++++++++++++++++
 test/app/fio.test.lua    | 21 ++++++++++++
 test/box/errinj.result   |  1 +
 5 files changed, 147 insertions(+), 11 deletions(-)

diff --git a/src/lib/core/coio_file.c b/src/lib/core/coio_file.c
index e2345567c..e290214bc 100644
--- a/src/lib/core/coio_file.c
+++ b/src/lib/core/coio_file.c
@@ -164,10 +164,30 @@ 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);
+	ssize_t left = count, pos = 0, res, chunk;
+	eio_req *req;
+
+	while (left > 0) {
+		INIT_COEIO_FILE(eio);
+		chunk = left;
+
+		ERROR_INJECT(ERRINJ_COIO_WRITE_CHUNK, {
+			chunk = 1;
+		});
+
+		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) {
+			pos = -1;
+			break;
+		} else {
+			left -= res;
+			pos += res;
+		}
+	}
+	return pos;
 }
 
 ssize_t
@@ -201,6 +221,11 @@ static void
 coio_do_write(eio_req *req)
 {
 	struct coio_file_task *eio = (struct coio_file_task *)req->data;
+
+	ERROR_INJECT(ERRINJ_COIO_WRITE_CHUNK, {
+		eio->write.count = 1;
+	});
+
 	req->result = write(eio->write.fd, eio->write.buf, eio->write.count);
 	eio->errorno = errno;
 }
@@ -208,13 +233,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) {
+			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 7577ed11a..e95611c1e 100644
--- a/src/lib/core/errinj.h
+++ b/src/lib/core/errinj.h
@@ -140,6 +140,7 @@ struct errinj {
 	_(ERRINJ_RELAY_FASTER_THAN_TX, ERRINJ_BOOL, {.bparam = false}) \
 	_(ERRINJ_INDEX_RESERVE, ERRINJ_BOOL, {.bparam = false})\
 	_(ERRINJ_TXN_COMMIT_ASYNC, ERRINJ_BOOL, {.bparam = false})\
+	_(ERRINJ_COIO_WRITE_CHUNK, ERRINJ_BOOL, {.bparam = false}) \
 
 ENUM0(errinj_id, ERRINJ_LIST);
 extern struct errinj errinjs[];
diff --git a/test/app/fio.result b/test/app/fio.result
index 783fa4fab..73fbd29e5 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', true)
+---
+- ok
+...
+fh7:write("one byte transfer, write")
+---
+- true
+...
+errinj.set('ERRINJ_COIO_WRITE_CHUNK', false)
+---
+- 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', true)
+---
+- 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', false)
+---
+- 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 6825b882f..733e0f25f 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', true)
+fh7:write("one byte transfer, write")
+errinj.set('ERRINJ_COIO_WRITE_CHUNK', false)
+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', true)
+fh8:pwrite("one byte transfer, ", 0)
+fh8:seek(0)
+fh8:read()
+fh8:pwrite("pwrite", 19)
+errinj.set('ERRINJ_COIO_WRITE_CHUNK', false)
+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 de877b708..6b4892500 100644
--- a/test/box/errinj.result
+++ b/test/box/errinj.result
@@ -47,6 +47,7 @@ evals
   - ERRINJ_BUILD_INDEX_DELAY: false
   - ERRINJ_CHECK_FORMAT_DELAY: false
   - ERRINJ_COIO_SENDFILE_CHUNK: -1
+  - ERRINJ_COIO_WRITE_CHUNK: false
   - ERRINJ_DYN_MODULE_COUNT: 0
   - ERRINJ_FIBER_MADVISE: false
   - ERRINJ_FIBER_MPROTECT: -1
-- 
2.20.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-06-11 20:12 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-26 18:05 [Tarantool-patches] [PATCH] fio/coio: Handle partial writes Cyrill Gorcunov
2019-12-04  9:44 ` Cyrill Gorcunov
2020-04-21 21:39 [Tarantool-patches] [PATCH] fio/coio: handle " Cyrill Gorcunov
2020-05-06 17:08 ` Alexander Turenko
2020-05-15  9:00   ` Cyrill Gorcunov
2020-06-09 16:55     ` Alexander Turenko
2020-06-09 22:55     ` Vladislav Shpilevoy
2020-06-09 22:55 ` Vladislav Shpilevoy
2020-06-10  7:52   ` Cyrill Gorcunov
2020-06-11 19:36     ` Vladislav Shpilevoy
2020-06-11 19:43       ` Cyrill Gorcunov
2020-06-11 19:56         ` Vladislav Shpilevoy
2020-06-11 20:12           ` Cyrill Gorcunov
2020-06-11 19:56 ` Vladislav Shpilevoy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox