Synced flush backport #11417
Synced flush backport #11417
Conversation
| @@ -358,7 +410,7 @@ public void onResponse(PreSyncedFlushResponse response) { | |||
|
|
|||
| @Override | |||
| public void onFailure(Throwable e) { | |||
| logger.trace("{} error while performing pre synced flush on [{}], skipping", e, shardId, shard); | |||
| logger.trace("{} error while performing pre synced flush on [{}], skipping", shardId, e, shard); | |||
There was a problem hiding this comment.
this looks flipped and does not log the exception?
|
Thx @brwe for picking this up. Left some minor comments |
|
did not spot any huge differences on my side, except the same question @bleskes had about removing the FLUSH executor... apart from that LGTM |
|
ok, all done. I let the bwc tests run and found that there we also have to check if it is a bwc test or not when we call indexRandom(). pushed a fix for that. |
brwe
force-pushed
the
synced-flush-backport
branch
from
6456fdd
to
1ca1d73
Compare
brwe
force-pushed
the
synced-flush-backport
branch
from
1ca1d73
to
cde4b66
Compare
|
fixed the bwc test and added checks to make sure we actually have at least one current version node when we run the bwc tests. want to take another look? |
| // bwc test | ||
| if (cluster() instanceof CompositeTestCluster) { | ||
| CompositeTestCluster compositeTestCluster = (CompositeTestCluster) cluster(); | ||
| if (compositeTestCluster.numNewDataNodes() > 0) { |
There was a problem hiding this comment.
can we add a comment as to why this is needed here? I'm not sure it's trivial to figure out.
|
LGTM. Left one little comment. Feel free to push. |
brwe
force-pushed
the
synced-flush-backport
branch
from
cde4b66
to
d22e134
Compare
clintongormley
added
>feature
:Distributed/Recovery
Was relatively unproblematic, only SyncedFlushService was a little hairy