Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH 0/2] A few fixes for gc and xlog
@ 2021-03-10 20:47 Cyrill Gorcunov via Tarantool-patches
  2021-03-10 20:47 ` [Tarantool-patches] [PATCH 1/2] gc: use wide integer for schedule counting Cyrill Gorcunov via Tarantool-patches
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-03-10 20:47 UTC (permalink / raw)
  To: tml; +Cc: Vladislav Shpilevoy

While work on https://github.com/tarantool/tarantool/issues/5806
is still in progress I would like to share a few patches which
I think are useful. Please take a look.

branch gorcunov/gh-5806-xlog-gc

Cyrill Gorcunov (2):
  gc: use wide integer for schedule counting
  xlog: do not sort sole entry

 src/box/gc.c   | 6 +++---
 src/box/gc.h   | 2 +-
 src/box/xlog.c | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)


base-commit: 3a7c21023367424baa9bf2d4d823102cb2b3c751
-- 
2.29.2


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

* [Tarantool-patches] [PATCH 1/2] gc: use wide integer for schedule counting
  2021-03-10 20:47 [Tarantool-patches] [PATCH 0/2] A few fixes for gc and xlog Cyrill Gorcunov via Tarantool-patches
@ 2021-03-10 20:47 ` Cyrill Gorcunov via Tarantool-patches
  2021-03-14 16:30   ` Vladislav Shpilevoy via Tarantool-patches
  2021-03-10 20:47 ` [Tarantool-patches] [PATCH 2/2] xlog: do not sort sole entry Cyrill Gorcunov via Tarantool-patches
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-03-10 20:47 UTC (permalink / raw)
  To: tml; +Cc: Vladislav Shpilevoy

Currently we use unsined int for "cleanup" schedule
counting, this is safe while this routine is not
called too often. Still there is a chance to hit
a number wrap on code modification because there
is no strict rule on how to use this garbage collector.
Lets use wide integers instead, we have only one gc
instance and such approach eliminates potential problems
in future (actually this should had been done from the
beginning since the current gc code flow developed
without wrapping in mind).

In-scope-of #5806

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/box/gc.c | 6 +++---
 src/box/gc.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/box/gc.c b/src/box/gc.c
index 1f8cc818d..86c229c34 100644
--- a/src/box/gc.c
+++ b/src/box/gc.c
@@ -239,15 +239,15 @@ gc_cleanup_fiber_f(va_list ap)
 {
 	(void)ap;
 	while (!fiber_is_cancelled()) {
-		int delta = gc.cleanup_scheduled - gc.cleanup_completed;
+		int64_t delta = gc.cleanup_scheduled - gc.cleanup_completed;
 		if (delta == 0) {
 			/* No pending garbage collection. */
 			fiber_sleep(TIMEOUT_INFINITY);
 			continue;
 		}
-		assert(delta > 0);
 		gc_run_cleanup();
 		gc.cleanup_completed += delta;
+		assert(delta > 0 && gc.cleanup_completed > 0);
 		fiber_cond_signal(&gc.cleanup_cond);
 	}
 	return 0;
@@ -278,7 +278,7 @@ gc_schedule_cleanup(void)
 static void
 gc_wait_cleanup(void)
 {
-	unsigned scheduled = gc.cleanup_scheduled;
+	int64_t scheduled = gc.cleanup_scheduled;
 	while (gc.cleanup_completed < scheduled)
 		fiber_cond_wait(&gc.cleanup_cond);
 }
diff --git a/src/box/gc.h b/src/box/gc.h
index 829aaf479..2a568c5f9 100644
--- a/src/box/gc.h
+++ b/src/box/gc.h
@@ -146,7 +146,7 @@ struct gc_state {
 	 * sleep until @completed reaches the value of @scheduled
 	 * taken at that moment of time.
 	 */
-	unsigned cleanup_completed, cleanup_scheduled;
+	int64_t cleanup_completed, cleanup_scheduled;
 	/**
 	 * Set if there's a fiber making a checkpoint right now.
 	 */
-- 
2.29.2


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

* [Tarantool-patches] [PATCH 2/2] xlog: do not sort sole entry
  2021-03-10 20:47 [Tarantool-patches] [PATCH 0/2] A few fixes for gc and xlog Cyrill Gorcunov via Tarantool-patches
  2021-03-10 20:47 ` [Tarantool-patches] [PATCH 1/2] gc: use wide integer for schedule counting Cyrill Gorcunov via Tarantool-patches
@ 2021-03-10 20:47 ` Cyrill Gorcunov via Tarantool-patches
  2021-03-12 17:11 ` [Tarantool-patches] [PATCH 0/2] A few fixes for gc and xlog Serge Petrenko via Tarantool-patches
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-03-10 20:47 UTC (permalink / raw)
  To: tml; +Cc: Vladislav Shpilevoy

In case if there only one snapshot or xlog file
there is no need to call sorting procedure at all.

In-scope-of #5806

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 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 974f460be..89df7e485 100644
--- a/src/box/xlog.c
+++ b/src/box/xlog.c
@@ -588,7 +588,7 @@ xdir_scan(struct xdir *dir, bool is_dir_required)
 		signatures[s_count++] = signature;
 	}
 	/** Sort the list of files */
-	if (s_count > 0)
+	if (s_count > 1)
 		qsort(signatures, s_count, sizeof(*signatures), cmp_i64);
 	/**
 	 * Update the log dir index with the current state:
-- 
2.29.2


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

* Re: [Tarantool-patches] [PATCH 0/2] A few fixes for gc and xlog
  2021-03-10 20:47 [Tarantool-patches] [PATCH 0/2] A few fixes for gc and xlog Cyrill Gorcunov via Tarantool-patches
  2021-03-10 20:47 ` [Tarantool-patches] [PATCH 1/2] gc: use wide integer for schedule counting Cyrill Gorcunov via Tarantool-patches
  2021-03-10 20:47 ` [Tarantool-patches] [PATCH 2/2] xlog: do not sort sole entry Cyrill Gorcunov via Tarantool-patches
@ 2021-03-12 17:11 ` Serge Petrenko via Tarantool-patches
  2021-03-12 17:45   ` Cyrill Gorcunov via Tarantool-patches
  2021-03-15 23:21 ` Vladislav Shpilevoy via Tarantool-patches
  2021-03-16 13:29 ` Kirill Yukhin via Tarantool-patches
  4 siblings, 1 reply; 9+ messages in thread
From: Serge Petrenko via Tarantool-patches @ 2021-03-12 17:11 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: Vladislav Shpilevoy, tml



10.03.2021 23:47, Cyrill Gorcunov пишет:
> While work on https://github.com/tarantool/tarantool/issues/5806
> is still in progress I would like to share a few patches which
> I think are useful. Please take a look.
>
> branch gorcunov/gh-5806-xlog-gc
>
> Cyrill Gorcunov (2):
>    gc: use wide integer for schedule counting
>    xlog: do not sort sole entry
>
>   src/box/gc.c   | 6 +++---
>   src/box/gc.h   | 2 +-
>   src/box/xlog.c | 2 +-
>   3 files changed, 5 insertions(+), 5 deletions(-)
>
>
> base-commit: 3a7c21023367424baa9bf2d4d823102cb2b3c751
Thanks for the patchset!

LGTM, but I doubt Vlad will like this =)

-- 
Serge Petrenko


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

* Re: [Tarantool-patches] [PATCH 0/2] A few fixes for gc and xlog
  2021-03-12 17:11 ` [Tarantool-patches] [PATCH 0/2] A few fixes for gc and xlog Serge Petrenko via Tarantool-patches
@ 2021-03-12 17:45   ` Cyrill Gorcunov via Tarantool-patches
  0 siblings, 0 replies; 9+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-03-12 17:45 UTC (permalink / raw)
  To: Serge Petrenko; +Cc: Vladislav Shpilevoy, tml

On Fri, Mar 12, 2021 at 08:11:25PM +0300, Serge Petrenko wrote:
> 
> 10.03.2021 23:47, Cyrill Gorcunov пишет:
> > While work on https://github.com/tarantool/tarantool/issues/5806
> > is still in progress I would like to share a few patches which
> > I think are useful. Please take a look.
> > 
> > branch gorcunov/gh-5806-xlog-gc
> > 
> > Cyrill Gorcunov (2):
> >    gc: use wide integer for schedule counting
> >    xlog: do not sort sole entry
> > 
> >   src/box/gc.c   | 6 +++---
> >   src/box/gc.h   | 2 +-
> >   src/box/xlog.c | 2 +-
> >   3 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > 
> > base-commit: 3a7c21023367424baa9bf2d4d823102cb2b3c751
> Thanks for the patchset!
> 
> LGTM, but I doubt Vlad will like this =)

Thanks! Both fixes are trivial but definitely needed ones.

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

* Re: [Tarantool-patches] [PATCH 1/2] gc: use wide integer for schedule counting
  2021-03-10 20:47 ` [Tarantool-patches] [PATCH 1/2] gc: use wide integer for schedule counting Cyrill Gorcunov via Tarantool-patches
@ 2021-03-14 16:30   ` Vladislav Shpilevoy via Tarantool-patches
  2021-03-14 18:24     ` Cyrill Gorcunov via Tarantool-patches
  0 siblings, 1 reply; 9+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-03-14 16:30 UTC (permalink / raw)
  To: Cyrill Gorcunov, tml

Hi! Thanks for the patch!

> diff --git a/src/box/gc.c b/src/box/gc.c
> index 1f8cc818d..86c229c34 100644
> --- a/src/box/gc.c
> +++ b/src/box/gc.c
> @@ -239,15 +239,15 @@ gc_cleanup_fiber_f(va_list ap)
>  {
>  	(void)ap;
>  	while (!fiber_is_cancelled()) {
> -		int delta = gc.cleanup_scheduled - gc.cleanup_completed;
> +		int64_t delta = gc.cleanup_scheduled - gc.cleanup_completed;
>  		if (delta == 0) {
>  			/* No pending garbage collection. */
>  			fiber_sleep(TIMEOUT_INFINITY);
>  			continue;
>  		}
> -		assert(delta > 0);
>  		gc_run_cleanup();
>  		gc.cleanup_completed += delta;
> +		assert(delta > 0 && gc.cleanup_completed > 0);

You didn't need to change the assertion. If `delta` is always
positive, and `cleanup_completed` is populated only from `delta`,
it is also positive always.

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

* Re: [Tarantool-patches] [PATCH 1/2] gc: use wide integer for schedule counting
  2021-03-14 16:30   ` Vladislav Shpilevoy via Tarantool-patches
@ 2021-03-14 18:24     ` Cyrill Gorcunov via Tarantool-patches
  0 siblings, 0 replies; 9+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-03-14 18:24 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tml

On Sun, Mar 14, 2021 at 05:30:57PM +0100, Vladislav Shpilevoy wrote:
> > -		assert(delta > 0);
> >  		gc_run_cleanup();
> >  		gc.cleanup_completed += delta;
> > +		assert(delta > 0 && gc.cleanup_completed > 0);
> 
> You didn't need to change the assertion. If `delta` is always
> positive, and `cleanup_completed` is populated only from `delta`,
> it is also positive always.

This allows to make sure that gc.cleanup_completed didn't
change in gc_run_cleanup in unpredicted way. I'll update
if you prefer, sure thing. Force pushed.
---
diff --git a/src/box/gc.c b/src/box/gc.c
index 1f8cc818d..9af4ef958 100644
--- a/src/box/gc.c
+++ b/src/box/gc.c
@@ -239,7 +239,7 @@ gc_cleanup_fiber_f(va_list ap)
 {
 	(void)ap;
 	while (!fiber_is_cancelled()) {
-		int delta = gc.cleanup_scheduled - gc.cleanup_completed;
+		int64_t delta = gc.cleanup_scheduled - gc.cleanup_completed;
 		if (delta == 0) {
 			/* No pending garbage collection. */
 			fiber_sleep(TIMEOUT_INFINITY);
@@ -278,7 +278,7 @@ gc_schedule_cleanup(void)
 static void
 gc_wait_cleanup(void)
 {
-	unsigned scheduled = gc.cleanup_scheduled;
+	int64_t scheduled = gc.cleanup_scheduled;
 	while (gc.cleanup_completed < scheduled)
 		fiber_cond_wait(&gc.cleanup_cond);
 }
diff --git a/src/box/gc.h b/src/box/gc.h
index 829aaf479..2a568c5f9 100644
--- a/src/box/gc.h
+++ b/src/box/gc.h
@@ -146,7 +146,7 @@ struct gc_state {
 	 * sleep until @completed reaches the value of @scheduled
 	 * taken at that moment of time.
 	 */
-	unsigned cleanup_completed, cleanup_scheduled;
+	int64_t cleanup_completed, cleanup_scheduled;
 	/**
 	 * Set if there's a fiber making a checkpoint right now.
 	 */
-- 
2.30.2


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

* Re: [Tarantool-patches] [PATCH 0/2] A few fixes for gc and xlog
  2021-03-10 20:47 [Tarantool-patches] [PATCH 0/2] A few fixes for gc and xlog Cyrill Gorcunov via Tarantool-patches
                   ` (2 preceding siblings ...)
  2021-03-12 17:11 ` [Tarantool-patches] [PATCH 0/2] A few fixes for gc and xlog Serge Petrenko via Tarantool-patches
@ 2021-03-15 23:21 ` Vladislav Shpilevoy via Tarantool-patches
  2021-03-16 13:29 ` Kirill Yukhin via Tarantool-patches
  4 siblings, 0 replies; 9+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-03-15 23:21 UTC (permalink / raw)
  To: Cyrill Gorcunov, tml

Hi! Thanks for the patch!

LGTM.

> branch gorcunov/gh-5806-xlog-gc

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

* Re: [Tarantool-patches] [PATCH 0/2] A few fixes for gc and xlog
  2021-03-10 20:47 [Tarantool-patches] [PATCH 0/2] A few fixes for gc and xlog Cyrill Gorcunov via Tarantool-patches
                   ` (3 preceding siblings ...)
  2021-03-15 23:21 ` Vladislav Shpilevoy via Tarantool-patches
@ 2021-03-16 13:29 ` Kirill Yukhin via Tarantool-patches
  4 siblings, 0 replies; 9+ messages in thread
From: Kirill Yukhin via Tarantool-patches @ 2021-03-16 13:29 UTC (permalink / raw)
  To: Cyrill Gorcunov; +Cc: tml, Vladislav Shpilevoy

Hello,

On 10 мар 23:47, Cyrill Gorcunov wrote:
> While work on https://github.com/tarantool/tarantool/issues/5806
> is still in progress I would like to share a few patches which
> I think are useful. Please take a look.
> 
> branch gorcunov/gh-5806-xlog-gc

I've checked your patchset into 2.6, 2.7 and master.

--
Regards, Kirill Yukhin

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

end of thread, other threads:[~2021-03-16 13:29 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-10 20:47 [Tarantool-patches] [PATCH 0/2] A few fixes for gc and xlog Cyrill Gorcunov via Tarantool-patches
2021-03-10 20:47 ` [Tarantool-patches] [PATCH 1/2] gc: use wide integer for schedule counting Cyrill Gorcunov via Tarantool-patches
2021-03-14 16:30   ` Vladislav Shpilevoy via Tarantool-patches
2021-03-14 18:24     ` Cyrill Gorcunov via Tarantool-patches
2021-03-10 20:47 ` [Tarantool-patches] [PATCH 2/2] xlog: do not sort sole entry Cyrill Gorcunov via Tarantool-patches
2021-03-12 17:11 ` [Tarantool-patches] [PATCH 0/2] A few fixes for gc and xlog Serge Petrenko via Tarantool-patches
2021-03-12 17:45   ` Cyrill Gorcunov via Tarantool-patches
2021-03-15 23:21 ` Vladislav Shpilevoy via Tarantool-patches
2021-03-16 13:29 ` Kirill Yukhin via Tarantool-patches

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