[TEST] remove redundant tests and move to different suite #11666
Conversation
brwe
force-pushed
the
meta-data-zen
branch
3 times, most recently
from
68f20f7 to
f82e958
Compare
| String redNode = startDataNode("red"); | ||
|
|
||
| // create blue_index on blue_node and same for red | ||
| client().admin().cluster().health(clusterHealthRequest().waitForYellowStatus().waitForNodes("3")).get(); |
There was a problem hiding this comment.
you can use ensureStableCluster(3)
|
thanks fr the review! addressed all comments. want to have another look? |
| @Ignore("https://github.com/elastic/elasticsearch/issues/11665") | ||
| @Test | ||
| public void testIndicesDeleted() throws Exception { | ||
| configureCluster(3, 2); |
There was a problem hiding this comment.
I think you want configureCluster(2, 2) for the two master nodes.
There was a problem hiding this comment.
I changed this back because of this comment: #11666 (comment)
There was a problem hiding this comment.
Is the first parameter the number of master eligible nodes or the number of all node, including data nodes?
There was a problem hiding this comment.
ok, talked to @bleskes , (3, 2) is actually correct
|
This looks great. Left some minor comments. |
|
@bleskes I added a method now that lets me restart any node I want and made all other helpers I used before private again. Let me know if that is OK. |
| @Ignore("https://github.com/elastic/elasticsearch/issues/11665") | ||
| @Test | ||
| public void testIndicesDeleted() throws Exception { | ||
| configureCluster(2, 2); |
There was a problem hiding this comment.
i think we need a configureCluster(3, 2), no?
| assertThat(clusterStateResponse.getState().getMetaData().index("red_index").getState().name(), equalTo(IndexMetaData.State.OPEN.name())); | ||
| // restart again | ||
| logger.debug("relocating index..."); | ||
| client().admin().indices().prepareUpdateSettings(index).setSettings(Settings.builder().put(FilterAllocationDecider.INDEX_ROUTING_INCLUDE_GROUP + "_name", node1)).get(); |
There was a problem hiding this comment.
nit pick - can we start with node1 and relocate to node2 ? :)
|
Thx @brwe . This looks great. I left some minor comments. |
| @@ -841,7 +845,9 @@ public void isolatedUnicastNodes() throws Exception { | |||
| } | |||
|
|
|||
|
|
|||
| /** Test cluster join with issues in cluster state publishing * */ | |||
| /** | |||
| * Test cluster join with issues in cluster state publishing * | |||
|
@bleskes so, turns out there was a bug in the meta data write with closed indices, see brwe@f141911 . comment not so minor after all. |
|
LGTM. Thx @brwe . Do we also want/need to add a unit test for brwe@f141911 (opening a closed index) ? (as a different change) |
Some of the test for meta data are redundant. Also, since they somewhat test service disruptions (start master with empty data folder) we might move them to DiscoveryWithServiceDisruptionsTests. Also, this commit adds a test for elastic#11665
…vant indices to write meta state When an index is opened it will not be assigned to a node but also not have closed state anymore. Before we only checked if an index either is closed or assigned to the data node and therefore the change from close->open was not written.
[TEST] remove redundant tests and move to different suite
Yes, I think we should. I opened #12475 to track this. |
Some of the test for meta data are redundant. Also, since they
somewhat test service disruptions (start master with empty
data folder) we might move them to DiscoveryWithServiceDisruptionsTests.
Also, this commit adds a test for
#11665