Adding a db merge end to end test.

[alainjobart]

And fix all the bugs it un-covers.

@enisoc @michael-berlin

---

This change is

[michael-berlin]

LGTM

But I'm not happy with the degree of duplication in the test. I think it's time for a utils_resharding.py module or something similar.

---

Reviewed 6 of 6 files at r1.
Review status: all files reviewed at latest revision, 10 unresolved discussions.

---

test/merge_sharding.py, line 47 [r1] (raw file):
It could be a bit more intuitive that this is the merged shard e.g. call it shard_01_master ?

---

test/merge_sharding.py, line 57 [r1] (raw file):
You could put all these in a list and loop over it.

---

test/merge_sharding.py, line 164 [r1] (raw file):
Document here the return value?

---

test/merge_sharding.py, line 179 [r1] (raw file):
Is the comma after "custom_sharding_key)" necessary?

---

test/merge_sharding.py, line 211 [r1] (raw file):
Align the id with the shard numbers e.g.

self._insert_value(shard_0_master, 'resharding1', 0, 'msg1',

? Or:

self._insert_value(shard_1_master, 'resharding1', 1, 'msg1',

---

test/merge_sharding.py, line 235 [r1] (raw file):
You could add a check here that count + base must be < 10000 or there will be duplicate rows during the merge?

---

test/merge_sharding.py, line 236 [r1] (raw file):
Same comment here as above. I would prefer if the shard number is aligned with the content of the rows e.g.

self._insert_value(shard_1_master, 'resharding1', 1, 'msg1',

---

test/merge_sharding.py, line 263 [r1] (raw file):
We have utils.wait_step(). Maybe you can use that?

---

test/merge_sharding.py, line 273 [r1] (raw file):
This is copied from resharding.py.

Can you please deduplicate it?

I would like to avoid duplicate resharding test code as much as possible.

---

test/merge_sharding.py, line 304 [r1] (raw file):
Why not just start them with it?

You'll probably still want the explicit healthcheck here to not wait for the next periodic one.

---

Comments from Reviewable

[alainjobart]

Review status: all files reviewed at latest revision, 10 unresolved discussions.

---

test/merge_sharding.py, line 47 [r1] (raw file):
Done.

---

test/merge_sharding.py, line 57 [r1] (raw file):
Done.

---

test/merge_sharding.py, line 164 [r1] (raw file):
Done.

---

test/merge_sharding.py, line 179 [r1] (raw file):
Yes it is, to make it a tuple. python will remove () around a tuple if it's only one value:

print ('a')
a
print ('a',)
('a',)

---

test/merge_sharding.py, line 211 [r1] (raw file):
Done.

---

test/merge_sharding.py, line 235 [r1] (raw file):
Done.

---

test/merge_sharding.py, line 236 [r1] (raw file):
Done.

---

test/merge_sharding.py, line 263 [r1] (raw file):
Done.

---

test/merge_sharding.py, line 273 [r1] (raw file):
There is a lot to do in this regard. I will work on some of it.

---

test/merge_sharding.py, line 304 [r1] (raw file):
Exactly, we have to explicitly do it anyway. So there is little value in having the tablet do it manually.

---

Comments from Reviewable

[alainjobart]

@michael-berlin PTAL I factored out a bunch of stuff. We can still do better.

[michael-berlin]

---

Reviewed 2 of 2 files at r2, 6 of 6 files at r3, 5 of 5 files at r4, 5 of 5 files at r5, 5 of 5 files at r6.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

---

test/resharding.py, line 21 [r3] (raw file):
typo: SourceShardAdd
typo: SourceShardDelete

I suggest to add "vtctl" to be explicit about it:

• tests vtctl SourceShardAdd and SourceShardDelete.

---

test/utils.py, line 382 [r6] (raw file):
waits for vars[var][key]==value.

?

---

test/utils.py, line 383 [r6] (raw file):
waits for vars[var][key]==value.

?

---

Comments from Reviewable