[PATCH 0/2] vinyl: allow to limit dump bandwidth

[PATCH 0/2] vinyl: allow to limit dump bandwidth

[Vladimir Davydov]

Apply box.cfg.snap_io_rate_limit to vinyl dump/compaction tasks.

https://github.com/tarantool/tarantool/issues/3220
https://github.com/tarantool/tarantool/commits/gh-3220-vy-limit-dump-bandwidth

Vladimir Davydov (2):
  xlog: use ev_sleep instead of fiber_sleep for rate limiting
  vinyl: apply box.cfg.snap_io_rate_limit to dump/compaction

 src/box/box.cc                   |  5 +++
 src/box/vinyl.c                  |  6 +++
 src/box/vinyl.h                  |  6 +++
 src/box/vy_run.c                 |  7 +++-
 src/box/vy_run.h                 |  2 +
 src/box/xlog.c                   |  2 +-
 test/vinyl/snap_io_rate.result   | 85 ++++++++++++++++++++++++++++++++++++++++
 test/vinyl/snap_io_rate.test.lua | 38 ++++++++++++++++++
 8 files changed, 149 insertions(+), 2 deletions(-)
 create mode 100644 test/vinyl/snap_io_rate.result
 create mode 100644 test/vinyl/snap_io_rate.test.lua

-- 
2.11.0

[PATCH 1/2] xlog: use ev_sleep instead of fiber_sleep for rate limiting

[Vladimir Davydov]

fiber_sleep() works only if the current thread was created with
cord_costart(). Since vinyl worker threads don't need fibers, they
are created with cord_start() and hence can't use fiber_sleep().
So to be able to limit rate of vinyl dump/compaction, we have to
use ev_sleep() instead of fiber_sleep() in xlog. This is fine by
other xlog writers, because they don't use fibers either, neither
they should as xlogs are written without coio.

Needed for #3220
---
 src/box/xlog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/box/xlog.c b/src/box/xlog.c
index af79e324..be7b3459 100644
--- a/src/box/xlog.c
+++ b/src/box/xlog.c
@@ -1090,7 +1090,7 @@ xlog_tx_write(struct xlog *log)
 			throttle_time = (double)sync_len / log->rate_limit -
 					(ev_monotonic_time() - log->sync_time);
 			if (throttle_time > 0)
-				fiber_sleep(throttle_time);
+				ev_sleep(throttle_time);
 		}
 		/** sync data from cache to disk */
 #ifdef HAVE_SYNC_FILE_RANGE
-- 
2.11.0

Re: [PATCH 1/2] xlog: use ev_sleep instead of fiber_sleep for rate limiting

[Konstantin Osipov]

* Vladimir Davydov <vdavydov.dev@gmail.com> [18/05/29 18:20]:
> fiber_sleep() works only if the current thread was created with
> cord_costart(). Since vinyl worker threads don't need fibers, they
> are created with cord_start() and hence can't use fiber_sleep().
> So to be able to limit rate of vinyl dump/compaction, we have to
> use ev_sleep() instead of fiber_sleep() in xlog. This is fine by
> other xlog writers, because they don't use fibers either, neither
> they should as xlogs are written without coio.
> 
> Needed for #3220

Please use cord_costart() in vinyl workers instead.

We will end up needing it for other things and it's nearly free.

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

Re: [PATCH 1/2] xlog: use ev_sleep instead of fiber_sleep for rate limiting

[Vladimir Davydov]

On Fri, Jun 01, 2018 at 08:52:55PM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev@gmail.com> [18/05/29 18:20]:
> > fiber_sleep() works only if the current thread was created with
> > cord_costart(). Since vinyl worker threads don't need fibers, they
> > are created with cord_start() and hence can't use fiber_sleep().
> > So to be able to limit rate of vinyl dump/compaction, we have to
> > use ev_sleep() instead of fiber_sleep() in xlog. This is fine by
> > other xlog writers, because they don't use fibers either, neither
> > they should as xlogs are written without coio.
> > 
> > Needed for #3220
> 
> Please use cord_costart() in vinyl workers instead.

For this to work, we would need to yield periodically (fiber_sleep(0))
while writing a run file. This would look weird as each worker thread
executes just one task.

Anyway, yielding in xlog_write() may be unexpected from caller's pov,
because this function is blocking (writes a file) and isn't supposed
to yield. One usually hands it over to coio or a worker thread, where
yielding is pointless.

> 
> We will end up needing it for other things and it's nearly free.

What things?

Re: [PATCH 1/2] xlog: use ev_sleep instead of fiber_sleep for rate limiting

[Konstantin Osipov]

* Vladimir Davydov <vdavydov.dev@gmail.com> [18/06/04 23:48]:
> Anyway, yielding in xlog_write() may be unexpected from caller's pov,
> because this function is blocking (writes a file) and isn't supposed
> to yield. One usually hands it over to coio or a worker thread, where
> yielding is pointless.
> 
> > 
> > We will end up needing it for other things and it's nearly free.
> 
> What things?

Multiple things going on in the writer thread. Fiber-based
programming is a powerful concept, and we're already using cbus
for writer threads. Any kind of monitoring, collecting statistics,
periodic work is easier done when you have fiber runtime
available. 

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

[PATCH 2/2] vinyl: apply box.cfg.snap_io_rate_limit to dump/compaction

[Vladimir Davydov]

Vinyl worker threads can consume all disk bandwidth while performing
dump or compaction, thus stalling DML requests, which also need some
disk bandwidth for WAL. Memtx has a similar problem - it needs to write
snapshot files. In case of memtx, we cope with this problem by limiting
the write rate with box.cfg.snap_io_rate_limit option. Let's reuse this
option for limiting vinyl dump/compaction rate.

Closes #3220
---
 src/box/box.cc                   |  5 +++
 src/box/vinyl.c                  |  6 +++
 src/box/vinyl.h                  |  6 +++
 src/box/vy_run.c                 |  7 +++-
 src/box/vy_run.h                 |  2 +
 test/vinyl/snap_io_rate.result   | 85 ++++++++++++++++++++++++++++++++++++++++
 test/vinyl/snap_io_rate.test.lua | 38 ++++++++++++++++++
 7 files changed, 148 insertions(+), 1 deletion(-)
 create mode 100644 test/vinyl/snap_io_rate.result
 create mode 100644 test/vinyl/snap_io_rate.test.lua

diff --git a/src/box/box.cc b/src/box/box.cc
index ac30eb4d..91c5f9f2 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -698,6 +698,11 @@ box_set_snap_io_rate_limit(void)
 	assert(memtx != NULL);
 	memtx_engine_set_snap_io_rate_limit(memtx,
 			cfg_getd("snap_io_rate_limit"));
+	struct vinyl_engine *vinyl;
+	vinyl = (struct vinyl_engine *)engine_by_name("vinyl");
+	assert(vinyl != NULL);
+	vinyl_engine_set_snap_io_rate_limit(vinyl,
+			cfg_getd("snap_io_rate_limit"));
 }
 
 void
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index f0d26874..2abe5696 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -2787,6 +2787,12 @@ vinyl_engine_set_too_long_threshold(struct vinyl_engine *vinyl,
 	vinyl->env->lsm_env.too_long_threshold = too_long_threshold;
 }
 
+void
+vinyl_engine_set_snap_io_rate_limit(struct vinyl_engine *vinyl, double limit)
+{
+	vinyl->env->run_env.snap_io_rate_limit = limit * 1024 * 1024;
+}
+
 /** }}} Environment */
 
 /* {{{ Checkpoint */
diff --git a/src/box/vinyl.h b/src/box/vinyl.h
index ac7afefb..9fec3d0a 100644
--- a/src/box/vinyl.h
+++ b/src/box/vinyl.h
@@ -76,6 +76,12 @@ void
 vinyl_engine_set_too_long_threshold(struct vinyl_engine *vinyl,
 				    double too_long_threshold);
 
+/**
+ * Update snap_io_rate_limit.
+ */
+void
+vinyl_engine_set_snap_io_rate_limit(struct vinyl_engine *vinyl, double limit);
+
 #ifdef __cplusplus
 } /* extern "C" */
 
diff --git a/src/box/vy_run.c b/src/box/vy_run.c
index 1011abce..e2edbcaa 100644
--- a/src/box/vy_run.c
+++ b/src/box/vy_run.c
@@ -1956,6 +1956,8 @@ vy_run_write_index(struct vy_run *run, const char *dirpath,
 	if (xlog_create(&index_xlog, path, 0, &meta) < 0)
 		return -1;
 
+	index_xlog.rate_limit = run->env->snap_io_rate_limit;
+
 	xlog_tx_begin(&index_xlog);
 
 	struct xrow_header xrow;
@@ -2035,7 +2037,10 @@ vy_run_writer_create_xlog(struct vy_run_writer *writer)
 		.filetype = XLOG_META_TYPE_RUN,
 		.instance_uuid = INSTANCE_UUID,
 	};
-	return xlog_create(&writer->data_xlog, path, 0, &meta);
+	if (xlog_create(&writer->data_xlog, path, 0, &meta) != 0)
+		return -1;
+	writer->data_xlog.rate_limit = writer->run->env->snap_io_rate_limit;
+	return 0;
 }
 
 /**
diff --git a/src/box/vy_run.h b/src/box/vy_run.h
index 6551191b..7bafffec 100644
--- a/src/box/vy_run.h
+++ b/src/box/vy_run.h
@@ -54,6 +54,8 @@ struct vy_run_reader;
 
 /** Part of vinyl environment for run read/write */
 struct vy_run_env {
+	/** Write rate limit, in bytes per second. */
+	uint64_t snap_io_rate_limit;
 	/** Mempool for struct vy_page_read_task */
 	struct mempool read_task_pool;
 	/** Key for thread-local ZSTD context */
diff --git a/test/vinyl/snap_io_rate.result b/test/vinyl/snap_io_rate.result
new file mode 100644
index 00000000..ecc8d35c
--- /dev/null
+++ b/test/vinyl/snap_io_rate.result
@@ -0,0 +1,85 @@
+fiber = require('fiber')
+---
+...
+digest = require('digest')
+---
+...
+test_run = require('test_run').new()
+---
+...
+MB = 1024 * 1024
+---
+...
+TUPLE_SIZE = 1024
+---
+...
+TUPLE_COUNT = 100
+---
+...
+snap_io_rate_limit = box.cfg.snap_io_rate_limit
+---
+...
+box.cfg{snap_io_rate_limit = 0.1}
+---
+...
+s = box.schema.space.create('test', {engine = 'vinyl'})
+---
+...
+_ = s:create_index('primary', {page_size = TUPLE_SIZE, run_count_per_level = 1, run_size_ratio = 10})
+---
+...
+function fill() for i = 1, TUPLE_COUNT do s:replace{i, digest.urandom(TUPLE_SIZE)} end end
+---
+...
+-- check that snap_io_rate_limit is applied to dump
+fill()
+---
+...
+t1 = fiber.time()
+---
+...
+box.snapshot()
+---
+- ok
+...
+t2 = fiber.time()
+---
+...
+rate = TUPLE_SIZE * TUPLE_COUNT / (t2 - t1) / MB
+---
+...
+rate < box.cfg.snap_io_rate_limit or rate
+---
+- true
+...
+-- check that snap_io_rate_limit is applied to compaction
+fill()
+---
+...
+t1 = fiber.time()
+---
+...
+box.snapshot()
+---
+- ok
+...
+while s.index.primary:info().disk.compact.count == 0 do fiber.sleep(0.001) end
+---
+...
+t2 = fiber.time()
+---
+...
+-- dump + compaction => multiply by 2
+rate = 2 * TUPLE_SIZE * TUPLE_COUNT / (t2 - t1) / MB
+---
+...
+rate < box.cfg.snap_io_rate_limit or rate
+---
+- true
+...
+s:drop()
+---
+...
+box.cfg{snap_io_rate_limit = snap_io_rate_limit}
+---
+...
diff --git a/test/vinyl/snap_io_rate.test.lua b/test/vinyl/snap_io_rate.test.lua
new file mode 100644
index 00000000..836bf537
--- /dev/null
+++ b/test/vinyl/snap_io_rate.test.lua
@@ -0,0 +1,38 @@
+fiber = require('fiber')
+digest = require('digest')
+test_run = require('test_run').new()
+
+MB = 1024 * 1024
+TUPLE_SIZE = 1024
+TUPLE_COUNT = 100
+
+snap_io_rate_limit = box.cfg.snap_io_rate_limit
+box.cfg{snap_io_rate_limit = 0.1}
+
+s = box.schema.space.create('test', {engine = 'vinyl'})
+_ = s:create_index('primary', {page_size = TUPLE_SIZE, run_count_per_level = 1, run_size_ratio = 10})
+
+function fill() for i = 1, TUPLE_COUNT do s:replace{i, digest.urandom(TUPLE_SIZE)} end end
+
+-- check that snap_io_rate_limit is applied to dump
+fill()
+t1 = fiber.time()
+box.snapshot()
+t2 = fiber.time()
+
+rate = TUPLE_SIZE * TUPLE_COUNT / (t2 - t1) / MB
+rate < box.cfg.snap_io_rate_limit or rate
+
+-- check that snap_io_rate_limit is applied to compaction
+fill()
+t1 = fiber.time()
+box.snapshot()
+while s.index.primary:info().disk.compact.count == 0 do fiber.sleep(0.001) end
+t2 = fiber.time()
+
+-- dump + compaction => multiply by 2
+rate = 2 * TUPLE_SIZE * TUPLE_COUNT / (t2 - t1) / MB
+rate < box.cfg.snap_io_rate_limit or rate
+
+s:drop()
+box.cfg{snap_io_rate_limit = snap_io_rate_limit}
-- 
2.11.0

Re: [PATCH 2/2] vinyl: apply box.cfg.snap_io_rate_limit to dump/compaction

[Konstantin Osipov]

* Vladimir Davydov <vdavydov.dev@gmail.com> [18/05/29 18:20]:

> diff --git a/test/vinyl/snap_io_rate.test.lua b/test/vinyl/snap_io_rate.test.lua
> new file mode 100644
> index 00000000..836bf537
> --- /dev/null
> +++ b/test/vinyl/snap_io_rate.test.lua
> @@ -0,0 +1,38 @@
> +fiber = require('fiber')
> +digest = require('digest')
> +test_run = require('test_run').new()
> +
> +MB = 1024 * 1024
> +TUPLE_SIZE = 1024
> +TUPLE_COUNT = 100

This test runs for 3.8 seconds on my laptop.

You only need to run it for 0.2 seconds, definitely 1 second to
test the rate.

Sorry for being painfully finicky about this, but I run tests
every day.

If you hate this request, don't bother, this is a standalone test
and can run in parallel.


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

Re: [PATCH 2/2] vinyl: apply box.cfg.snap_io_rate_limit to dump/compaction

[Vladimir Davydov]

On Fri, Jun 01, 2018 at 08:56:28PM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev@gmail.com> [18/05/29 18:20]:
> 
> > diff --git a/test/vinyl/snap_io_rate.test.lua b/test/vinyl/snap_io_rate.test.lua
> > new file mode 100644
> > index 00000000..836bf537
> > --- /dev/null
> > +++ b/test/vinyl/snap_io_rate.test.lua
> > @@ -0,0 +1,38 @@
> > +fiber = require('fiber')
> > +digest = require('digest')
> > +test_run = require('test_run').new()
> > +
> > +MB = 1024 * 1024
> > +TUPLE_SIZE = 1024
> > +TUPLE_COUNT = 100
> 
> This test runs for 3.8 seconds on my laptop.
> 
> You only need to run it for 0.2 seconds, definitely 1 second to
> test the rate.

xlog_write() throttles after writing a file for one second. So in order
to make sure throttling works as expected, we need to run a test for a
few seconds. To speed up the test, we need to rewrite the way throttling
works, but I don't think it's worth it.

> 
> Sorry for being painfully finicky about this, but I run tests
> every day.
> 
> If you hate this request, don't bother, this is a standalone test
> and can run in parallel.

Exactly.