[tarantool-patches] [PATCH] recovery: build secondary index in hot standby mode

[tarantool-patches] [PATCH] recovery: build secondary index in hot standby mode

[Ilya Kosarev]

Secondary index was being built after reading xlog
except for the force recovery case,
when it was being built immediately after reading snapshot.
Now it is also being built immediately after reading snapshot
in case of hot standby mode in order to be ready by the time
original instance quits.

Closes #4135
---
Branch: https://github.com/tarantool/tarantool/tree/i.kosarev/gh-4135-secondary-index-for-hot-standby
Issue: https://github.com/tarantool/tarantool/issues/4135

 src/box/memtx_engine.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c
index 7c3dd8465..2c8653bd9 100644
--- a/src/box/memtx_engine.c
+++ b/src/box/memtx_engine.c
@@ -48,6 +48,7 @@
 #include "replication.h"
 #include "schema.h"
 #include "gc.h"
+#include "wal.h"
 
 /* sync snapshot every 16MB */
 #define SNAP_SYNC_INTERVAL	(1 << 24)
@@ -269,7 +270,7 @@ memtx_engine_begin_final_recovery(struct engine *engine)
 	/* End of the fast path: loaded the primary key. */
 	space_foreach(memtx_end_build_primary_key, memtx);
 
-	if (!memtx->force_recovery) {
+	if (!memtx->force_recovery && wal_dir_lock >= 0) {
 		/*
 		 * Fast start path: "play out" WAL
 		 * records using the primary key only,
-- 
2.17.1

[tarantool-patches] Re: [PATCH] recovery: build secondary index in hot standby mode

[Konstantin Osipov]

* Ilya Kosarev <i.kosarev@tarantool.org> [19/08/14 21:36]:
> Secondary index was being built after reading xlog
> except for the force recovery case,
> when it was being built immediately after reading snapshot.
> Now it is also being built immediately after reading snapshot
> in case of hot standby mode in order to be ready by the time
> original instance quits.
> 
> Closes #4135
> ---
> Branch: https://github.com/tarantool/tarantool/tree/i.kosarev/gh-4135-secondary-index-for-hot-standby
> Issue: https://github.com/tarantool/tarantool/issues/4135
> 
>  src/box/memtx_engine.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c
> index 7c3dd8465..2c8653bd9 100644
> --- a/src/box/memtx_engine.c
> +++ b/src/box/memtx_engine.c
> @@ -48,6 +48,7 @@
>  #include "replication.h"
>  #include "schema.h"
>  #include "gc.h"
> +#include "wal.h"

This should not be done by including entire WAL API header..

I wonder maybe we should have a policy that discourages including
extra headers?

-- 
Konstantin Osipov, Moscow, Russia

[tarantool-patches] Re: [PATCH] recovery: build secondary index in hot standby mode

[Georgy Kirichenko]

[-- Attachment #1: Type: text/plain, Size: 598 bytes --]

On Wednesday, August 14, 2019 9:31:44 PM MSK Ilya Kosarev wrote:
Hi, thanks for you patch!
Please see my comments below
>...
>  #include "gc.h"
> +#include "wal.h"
This breaks encapsulation and logic - a wal isn't the object the engine 
interacts with.
> 
>  /* sync snapshot every 16MB */
>...
> 
> -	if (!memtx->force_recovery) {
> +	if (!memtx->force_recovery && wal_dir_lock >= 0) {
The same mistake, it isn't a memtx deal - is there any wal dir and was it 
locked or not.
>...  		/*

I pushed different solution onto your branch, please investigate it.
Also there should be some tests I think.

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]