Remove NoneGateway, NoneGatewayAllocator, & NoneGatewayModule #8537
Conversation
|
I haven't started looking but I already like the title and the diff stats. :-) |
|
This looks good to me, maybe we could try to have a static variable in ElasticsearchTestCase holding a no-op gateway allocator so that we don't have to create a new anonymous instance whenever we need it? Would also be nice if someone else could give it a look since it's a part of elasticsearch that I don't kno well. |
| @@ -354,7 +354,22 @@ public void testNoRebalanceOnPrimaryOverload() { | |||
| ImmutableSettings.Builder settings = settingsBuilder(); | |||
| AllocationService strategy = new AllocationService(settings.build(), randomAllocationDeciders(settings.build(), | |||
| new NodeSettingsService(ImmutableSettings.Builder.EMPTY_SETTINGS), getRandom()), new ShardsAllocators(settings.build(), | |||
| new NoneGatewayAllocator(), new ShardsAllocator() { | |||
| new GatewayAllocator() { | |||
There was a problem hiding this comment.
Can we make a NoopGatewayAllocator (alla NoopClusterService) and put it in org.elasticsearch.test.gateway? then we can reuse it bellow
|
LGTM. Left some minor comments + a question about removed test. |
|
nice work @dakrone ! I just bumped into the gateway documentation, I think it should be updated as part of this PR as |
|
One more thing, if I understand correctly the |
|
@javanna good catch! I've updated the documentation and The setting remains configurable, but only for internal (plugin) use only. |
+1
+1 (we may want to mock it at some point) Left one minor comment LGTM (with the change to elasticsearch.yml) |
dakrone
force-pushed
the
remove-none-gateway
branch
7 times, most recently
from
c98b0b9
to
9efdd83
Compare
Always use the LocalGateway* equivalents We already check in the LocalGateway whether a node is a client node, or is not master-eligible, and skip writing the state there. This allows us to remove this code that was previously used only for tribe nodes (which are not master eligible anyway and wouldn't write state) and in tests (which can shake more bugs out)
dakrone
force-pushed
the
remove-none-gateway
branch
from
9efdd83
to
e482bdd
Compare
|
Merged to master |
|
w00t |
Always use the LocalGateway* equivalents
The reason of this change is that we already check in the
LocalGatewaywhether a node is a client node, or is not master-eligible, and skip writing the state there. This allows us to remove this code that was previously used only for tribe nodes (which are not master eligible anyway and wouldn't write state) and in tests (which can shake more bugs out).