Skip to content

Commit

Permalink
Unmute RollupActionSingleNodeTests
Browse files Browse the repository at this point in the history
this commit unmutes the tests so that more CI runs with
new code that includes better error messages will highlight why
tests are flaky.

relates elastic#69506, elastic#69799, elastic#69733.
  • Loading branch information
talevy committed Mar 17, 2021
1 parent 8a9055a commit cbcbbea
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
/**
* The master rollup action that coordinates
* - creating rollup temporary index
* - calling {@link TransportRollupIndexerAction} to index rolluped up documents
* - calling {@link TransportRollupIndexerAction} to index rollup-ed documents
* - cleaning up state
*/
public class TransportRollupAction extends AcknowledgedTransportMasterNodeAction<RollupAction.Request> {
Expand Down Expand Up @@ -139,9 +139,11 @@ protected void masterOperation(Task task, RollupAction.Request request, ClusterS
// 3. run rollup indexer
// 4. make temp index read-only
// 5. shrink index
// 6. delete temporary index
// 6. publish rollup metadata and add rollup index to datastream
// 7. delete temporary index
// at any point if there is an issue, then cleanup temp index

// 1.
client.fieldCaps(fieldCapsRequest, ActionListener.wrap(fieldCapsResponse -> {
RollupActionRequestValidationException validationException = new RollupActionRequestValidationException();
if (fieldCapsResponse.get().size() == 0) {
Expand All @@ -156,6 +158,7 @@ protected void masterOperation(Task task, RollupAction.Request request, ClusterS
return;
}

// 2.
clusterService.submitStateUpdateTask("rollup create index", new ClusterStateUpdateTask() {
@Override
public ClusterState execute(ClusterState currentState) throws Exception {
Expand All @@ -170,12 +173,16 @@ public ClusterState execute(ClusterState currentState) throws Exception {

public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) {
// index created
// 3.
client.execute(RollupIndexerAction.INSTANCE, rollupIndexerRequest, ActionListener.wrap(indexerResp -> {
if (indexerResp.isCreated()) {
// 4.
client.admin().indices().updateSettings(updateSettingsReq, ActionListener.wrap(updateSettingsResponse -> {
if (updateSettingsResponse.isAcknowledged()) {
// 5.
client.admin().indices().resizeIndex(resizeRequest, ActionListener.wrap(resizeResponse -> {
if (resizeResponse.isAcknowledged()) {
// 6.
publishMetadata(originalIndexName, tmpIndexName, rollupIndexName, listener);
} else {
deleteTmpIndex(originalIndexName, tmpIndexName, listener,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
*/
package org.elasticsearch.xpack.rollup.v2;

import org.apache.lucene.util.LuceneTestCase;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.ResourceAlreadyExistsException;
import org.elasticsearch.action.ActionListener;
Expand Down Expand Up @@ -83,7 +82,6 @@
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;

@LuceneTestCase.AwaitsFix(bugUrl="https://github.com/elastic/elasticsearch/issues/69799")
public class RollupActionSingleNodeTests extends ESSingleNodeTestCase {

private static final DateFormatter DATE_FORMATTER = DateFormatter.forPattern("yyyy-MM-dd'T'HH:mm:ss.SSSXXX");
Expand Down Expand Up @@ -130,7 +128,6 @@ public void tearDown() throws Exception {
super.tearDown();
}

@AwaitsFix(bugUrl="https://github.com/elastic/elasticsearch/issues/69506")
public void testRollupShardIndexerCleansTempFiles() throws IOException {
// create rollup config and index documents into source index
RollupActionDateHistogramGroupConfig dateHistogramGroupConfig = randomRollupActionDateHistogramGroupConfig("date_1");
Expand All @@ -150,7 +147,7 @@ public void testRollupShardIndexerCleansTempFiles() throws IOException {
IndexShard shard = indexService.getShard(0);

// re-use source index as temp index for test
RollupShardIndexer indexer = new RollupShardIndexer(client(), indexService, shard.shardId(), config, index, 2);
RollupShardIndexer indexer = new RollupShardIndexer(client(), indexService, shard.shardId(), config, "tmp_" + index, 2);
indexer.execute();
assertThat(indexer.tmpFilesDeleted, equalTo(indexer.tmpFiles));
// assert that files are deleted
Expand All @@ -173,21 +170,14 @@ public void testCannotRollupToExistingIndex() throws Exception {
assertThat(exception.getMessage(), containsString("Unable to rollup index [" + index + "]"));
}

@AwaitsFix(bugUrl="https://github.com/elastic/elasticsearch/issues/69506")
public void testTemporaryIndexDeletedOnRollupFailure() throws Exception {
public void testTemporaryIndexCannotBeCreatedAlreadyExists() {
RollupActionDateHistogramGroupConfig dateHistogramGroupConfig = randomRollupActionDateHistogramGroupConfig("date_1");
SourceSupplier sourceSupplier = () -> XContentFactory.jsonBuilder().startObject()
.field("date_1", randomDateForInterval(dateHistogramGroupConfig.getInterval()))
.field("categorical_1", randomAlphaOfLength(1))
.endObject();
RollupActionConfig config = new RollupActionConfig(
new RollupActionGroupConfig(dateHistogramGroupConfig, null, new TermsGroupConfig("categorical_1")),
Collections.singletonList(new MetricConfig("numeric_non_existent", Collections.singletonList("max"))));
bulkIndex(sourceSupplier);
expectThrows(ElasticsearchException.class, () -> rollup(index, rollupIndex, config));
// assert that temporary index was removed
expectThrows(IndexNotFoundException.class,
() -> client().admin().indices().prepareGetIndex().addIndices(".rolluptmp-" + rollupIndex).get());
Collections.singletonList(new MetricConfig("numeric_1", Collections.singletonList("max"))));
assertTrue(client().admin().indices().prepareCreate(".rolluptmp-" + rollupIndex).get().isAcknowledged());
Exception exception = expectThrows(ElasticsearchException.class, () -> rollup(index, rollupIndex, config));
assertThat(exception.getMessage(), containsString("already exists"));
}

public void testCannotRollupWhileOtherRollupInProgress() throws Exception {
Expand Down

0 comments on commit cbcbbea

Please sign in to comment.