Skip to content

Deprecate freeze index API #72618

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 23 commits into from
May 27, 2021
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ public class IndicesClientIT extends ESRestHighLevelClientTestCase {
public static final RequestOptions LEGACY_TEMPLATE_OPTIONS = RequestOptions.DEFAULT.toBuilder()
.setWarningsHandler(warnings -> List.of(RestPutIndexTemplateAction.DEPRECATION_WARNING).equals(warnings) == false).build();

public static final String FROZEN_INDICES_DEPRECATION_WARNING = "Frozen indices are deprecated because they provide no benefit given " +
"improvements in heap memory utilization. They will be removed in a future release.";

public void testIndicesExists() throws IOException {
// Index present
{
Expand Down Expand Up @@ -1530,8 +1533,11 @@ public void testFreezeAndUnfreeze() throws IOException {
createIndex("test", Settings.EMPTY);
RestHighLevelClient client = highLevelClient();

final RequestOptions freezeIndexOptions = RequestOptions.DEFAULT.toBuilder()
.setWarningsHandler(warnings -> List.of(FROZEN_INDICES_DEPRECATION_WARNING).equals(warnings) == false).build();

ShardsAcknowledgedResponse freeze = execute(new FreezeIndexRequest("test"), client.indices()::freeze,
client.indices()::freezeAsync);
client.indices()::freezeAsync, freezeIndexOptions);
assertTrue(freeze.isShardsAcknowledged());
assertTrue(freeze.isAcknowledged());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;

import static org.elasticsearch.client.IndicesClientIT.FROZEN_INDICES_DEPRECATION_WARNING;
import static org.elasticsearch.client.IndicesClientIT.LEGACY_TEMPLATE_OPTIONS;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.equalTo;
Expand Down Expand Up @@ -2885,8 +2886,11 @@ public void testFreezeIndex() throws Exception {
request.setIndicesOptions(IndicesOptions.strictExpandOpen()); // <1>
// end::freeze-index-request-indicesOptions

final RequestOptions freezeIndexOptions = RequestOptions.DEFAULT.toBuilder()
.setWarningsHandler(warnings -> List.of(FROZEN_INDICES_DEPRECATION_WARNING).equals(warnings) == false).build();

// tag::freeze-index-execute
ShardsAcknowledgedResponse openIndexResponse = client.indices().freeze(request, RequestOptions.DEFAULT);
ShardsAcknowledgedResponse openIndexResponse = client.indices().freeze(request, freezeIndexOptions);
// end::freeze-index-execute

// tag::freeze-index-response
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.elasticsearch.action.support.ActiveShardCount;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.client.node.NodeClient;
import org.elasticsearch.common.RestApiVersion;
import org.elasticsearch.common.Strings;
import org.elasticsearch.protocol.xpack.frozen.FreezeRequest;
import org.elasticsearch.rest.BaseRestHandler;
Expand All @@ -22,11 +23,18 @@

public final class RestFreezeIndexAction extends BaseRestHandler {

public static final String DEPRECATION_WARNING = "Frozen indices are deprecated because they provide no benefit given improvements " +
"in heap memory utilization. They will be removed in a future release.";
private static final RestApiVersion DEPRECATION_VERSION = RestApiVersion.V_8;

@Override
public List<Route> routes() {
return List.of(
new Route(POST, "/{index}/_freeze"),
new Route(POST, "/{index}/_unfreeze"));
Route.builder(POST, "/{index}/_freeze")
.deprecated(DEPRECATION_WARNING, DEPRECATION_VERSION)
.build(),
Route.builder(POST, "/{index}/_unfreeze").build()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also deprecate this endpoint, because it will also technically be going away. If we don't show the message a user could hit this a bunch but not know it's going away.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was operating off the deprecation schedule listed in the meta issue in which unfreeze deprecation happens in 8.0. I'm open to deprecating it earlier though I'd like to get buy-in from at least some others involved in the meta issue.

Copy link
Contributor Author

@danhermann danhermann May 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zuketo, could you weigh in on the question of deprecating the unfreeze endpoint now versus in 8.0?

Edit: scratch that. We'll sort it out among the team.

);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@

public abstract class AbstractSearchableSnapshotsRestTestCase extends ESRestTestCase {

public static final String FROZEN_INDICES_WARNING = "Frozen indices are deprecated because they provide no benefit given "
+ "improvements in heap memory utilization. They will be removed in a future release.";

private static final String WRITE_REPOSITORY_NAME = "repository";
private static final String READ_REPOSITORY_NAME = "read-repository";
private static final String SNAPSHOT_NAME = "searchable-snapshot";
Expand Down Expand Up @@ -172,6 +175,7 @@ public void testSearchResults() throws Exception {
public void testSearchResultsWhenFrozen() throws Exception {
runSearchableSnapshotsTest((restoredIndexName, numDocs) -> {
final Request freezeRequest = new Request(HttpPost.METHOD_NAME, restoredIndexName + "/_freeze");
freezeRequest.setOptions(expectWarnings(FROZEN_INDICES_WARNING));
assertOK(client().performRequest(freezeRequest));
ensureGreen(restoredIndexName);
for (int i = 0; i < 10; i++) {
Expand Down Expand Up @@ -273,6 +277,7 @@ public void testSnapshotOfSearchableSnapshot() throws Exception {
if (frozen) {
logger.info("--> freezing index [{}]", restoredIndexName);
final Request freezeRequest = new Request(HttpPost.METHOD_NAME, restoredIndexName + "/_freeze");
freezeRequest.setOptions(expectWarnings(FROZEN_INDICES_WARNING));
assertOK(client().performRequest(freezeRequest));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
import java.util.List;
import java.util.Map;

import static org.elasticsearch.test.rest.ESRestTestCase.expectWarnings;

public class DataLoader {

public static void main(String[] args) throws Exception {
Expand Down Expand Up @@ -393,7 +395,14 @@ public static void makeAlias(RestClient client, String aliasName, String... indi

protected static void freeze(RestClient client, String... indices) throws Exception {
for (String index : indices) {
client.performRequest(new Request("POST", "/" + index + "/_freeze"));
Request freezeRequest = new Request("POST", "/" + index + "/_freeze");
freezeRequest.setOptions(
expectWarnings(
"Frozen indices are deprecated because they provide no benefit given improvements in "
+ "heap memory utilization. They will be removed in a future release."
)
);
client.performRequest(freezeRequest);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
---
"Basic":
- skip:
features: [ "allowed_warnings" ]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be "warnings" instead of "allowed_warnings" ? The difference is that if you only allow, then the warning can be missing and still pass the test. "warnings" is more like "require_warnings"


- do:
index:
index: test
Expand All @@ -12,6 +15,8 @@
body: { "foo": "Hello: 2" }

- do:
allowed_warnings:
- "Frozen indices are deprecated because they provide no benefit given improvements in heap memory utilization. They will be removed in a future release."
indices.freeze:
index: test

Expand Down Expand Up @@ -51,6 +56,8 @@


- do:
allowed_warnings:
- "Frozen indices are deprecated because they provide no benefit given improvements in heap memory utilization. They will be removed in a future release."
indices.freeze:
index: test*

Expand Down Expand Up @@ -102,6 +109,8 @@
- "the default value for the ?wait_for_active_shards parameter will change from '0' to 'index-setting' in version 8; specify '?wait_for_active_shards=index-setting' to adopt the future default behaviour, or '?wait_for_active_shards=0' to preserve today's behaviour"

- do:
allowed_warnings:
- "Frozen indices are deprecated because they provide no benefit given improvements in heap memory utilization. They will be removed in a future release."
indices.freeze:
index: test*,not_available
ignore_unavailable: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ setup:
---
"Translog stats on frozen indices":
- skip:
features: allowed_warnings
version: " - 7.3.99"
reason: "start ignoring translog retention policy with soft-deletes enabled in 7.4"

Expand Down Expand Up @@ -39,6 +40,8 @@ setup:

# freeze index
- do:
allowed_warnings:
- "Frozen indices are deprecated because they provide no benefit given improvements in heap memory utilization. They will be removed in a future release."
indices.freeze:
index: test
- is_true: acknowledged
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ setup:
---
"Usage stats on frozen indices":
- skip:
features: [ "allowed_warnings" ]
version: " - 7.3.99"
reason: "frozen indices have usage stats starting in version 7.4"

Expand Down Expand Up @@ -38,6 +39,8 @@ setup:

# freeze index
- do:
allowed_warnings:
- "Frozen indices are deprecated because they provide no benefit given improvements in heap memory utilization. They will be removed in a future release."
indices.freeze:
index: test
- is_true: acknowledged
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,14 @@ public void testFrozenIndexAfterRestarted() throws Exception {
ensureGreen(index);
final int totalHits = (int) XContentMapValues.extractValue("hits.total.value",
entityAsMap(client().performRequest(new Request("GET", "/" + index + "/_search"))));
assertOK(client().performRequest(new Request("POST", index + "/_freeze")));
Request freezeRequest = new Request("POST", index + "/_freeze");
freezeRequest.setOptions(
expectWarnings(
"Frozen indices are deprecated because they provide no benefit given "
+ "improvements in heap memory utilization. They will be removed in a future release."
)
);
assertOK(client().performRequest(freezeRequest));
ensureGreen(index);
assertNoFileBasedRecovery(index, n -> true);
final Request request = new Request("GET", "/" + index + "/_search");
Expand Down