Skip to content
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

Deletepit API #3

Open
wants to merge 34 commits into
base: createpit
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
34da6ea
fixing ci, adding tests and java docs
bharath-techie Apr 12, 2022
30cb6c0
Segregating create pit logic into separate controller
bharath-techie Apr 13, 2022
43fed74
Segregating create pit logic into separate controller
bharath-techie Apr 13, 2022
57232fb
Delete PIT API
bharath-techie Apr 13, 2022
ece4c00
Delete PIT API
bharath-techie Apr 18, 2022
d67e389
Delete PIT API changes
bharath-techie Apr 18, 2022
65c123a
Addressing review comments
bharath-techie Apr 29, 2022
a938e26
Merge branch 'createpit' of github.com:bharath-techie/OpenSearch into…
bharath-techie Apr 29, 2022
6789468
Merge branch 'main' of https://github.com/opensearch-project/OpenSear…
bharath-techie May 2, 2022
98451da
Merge branch 'main' of https://github.com/opensearch-project/OpenSear…
bharath-techie May 2, 2022
975b871
Merge branch 'main' of https://github.com/opensearch-project/OpenSear…
bharath-techie May 2, 2022
2a8e4ff
Merge branch 'createpit' of github.com:bharath-techie/OpenSearch into…
bharath-techie May 2, 2022
77be351
addressing review comments
bharath-techie May 2, 2022
849d1d3
Merge branch 'createpit' of github.com:bharath-techie/OpenSearch into…
bharath-techie May 2, 2022
51ce82f
addressing review comments
bharath-techie May 3, 2022
68e210d
Adding java docs and addressing comments
bharath-techie May 4, 2022
81ff93d
Merge branch 'createpit' of github.com:bharath-techie/OpenSearch into…
bharath-techie May 6, 2022
2acb465
changes to uniformly name pit
bharath-techie May 6, 2022
648402e
Addressing comments
bharath-techie May 10, 2022
480bdc2
Merge branch 'createpit' of github.com:bharath-techie/OpenSearch into…
bharath-techie May 11, 2022
04532f4
adding tests and comments
bharath-techie May 11, 2022
6ceaf61
addressing comments
bharath-techie May 13, 2022
fe4e41a
Merge branch 'createpit' of github.com:bharath-techie/OpenSearch into…
bharath-techie May 13, 2022
1dd62b4
Merge branch 'createpit' of github.com:bharath-techie/OpenSearch into…
bharath-techie May 17, 2022
af9b143
Rest high level clients
bharath-techie May 17, 2022
dc111f5
Merge branch 'createpit' of github.com:bharath-techie/OpenSearch into…
bharath-techie May 17, 2022
87cb8a5
Bump com.diffplug.spotless from 6.5.2 to 6.6.1 (#3356)
dependabot[bot] May 18, 2022
76b5ea6
Bump grpc-context from 1.45.1 to 1.46.0 in /plugins/repository-gcs (#…
dependabot[bot] May 18, 2022
892e984
[Type removal] Remove redundant _type in pipeline simulate action (#3…
dreamer-89 May 18, 2022
f8b102c
Removing hard coded value of max concurrent shard requests (#3364)
jainankitk May 18, 2022
248f188
fixing tests and adding util
bharath-techie May 18, 2022
829d79e
Merge branch 'createpit' of github.com:bharath-techie/OpenSearch into…
bharath-techie May 19, 2022
4a1cbd9
Create PIT API (#2745)
bharath-techie May 19, 2022
45a897f
Merge branch 'feature/point_in_time' of https://github.com/opensearch…
bharath-techie May 19, 2022
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
5 changes: 5 additions & 0 deletions server/src/main/java/org/opensearch/action/ActionModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -237,11 +237,13 @@
import org.opensearch.action.main.TransportMainAction;
import org.opensearch.action.search.ClearScrollAction;
import org.opensearch.action.search.CreatePITAction;
import org.opensearch.action.search.DeletePITAction;
import org.opensearch.action.search.MultiSearchAction;
import org.opensearch.action.search.SearchAction;
import org.opensearch.action.search.SearchScrollAction;
import org.opensearch.action.search.TransportClearScrollAction;
import org.opensearch.action.search.TransportCreatePITAction;
import org.opensearch.action.search.TransportDeletePITAction;
import org.opensearch.action.search.TransportMultiSearchAction;
import org.opensearch.action.search.TransportSearchAction;
import org.opensearch.action.search.TransportSearchScrollAction;
Expand Down Expand Up @@ -403,6 +405,7 @@
import org.opensearch.rest.action.search.RestClearScrollAction;
import org.opensearch.rest.action.search.RestCountAction;
import org.opensearch.rest.action.search.RestCreatePITAction;
import org.opensearch.rest.action.search.RestDeletePITAction;
import org.opensearch.rest.action.search.RestExplainAction;
import org.opensearch.rest.action.search.RestMultiSearchAction;
import org.opensearch.rest.action.search.RestSearchAction;
Expand Down Expand Up @@ -664,6 +667,7 @@ public <Request extends ActionRequest, Response extends ActionResponse> void reg
actions.register(DeleteDanglingIndexAction.INSTANCE, TransportDeleteDanglingIndexAction.class);
actions.register(FindDanglingIndexAction.INSTANCE, TransportFindDanglingIndexAction.class);
actions.register(CreatePITAction.INSTANCE, TransportCreatePITAction.class);
actions.register(DeletePITAction.INSTANCE, TransportDeletePITAction.class);

return unmodifiableMap(actions.getRegistry());
}
Expand Down Expand Up @@ -839,6 +843,7 @@ public void initRestHandlers(Supplier<DiscoveryNodes> nodesInCluster) {

// Point in time API
registerHandler.accept(new RestCreatePITAction());
registerHandler.accept(new RestDeletePITAction());
for (ActionPlugin plugin : actionPlugins) {
for (RestHandler handler : plugin.getRestHandlers(
settings,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.action.search;

import org.opensearch.action.ActionType;

public class DeletePITAction extends ActionType<DeletePITResponse> {

public static final DeletePITAction INSTANCE = new DeletePITAction();
public static final String NAME = "indices:admin/delete/pit";

private DeletePITAction() {
super(NAME, DeletePITResponse::new);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.action.search;

import org.opensearch.action.ActionRequest;
import org.opensearch.action.ActionRequestValidationException;
import org.opensearch.common.io.stream.StreamInput;
import org.opensearch.common.io.stream.StreamOutput;
import org.opensearch.common.xcontent.ToXContent;
import org.opensearch.common.xcontent.ToXContentObject;
import org.opensearch.common.xcontent.XContentBuilder;
import org.opensearch.common.xcontent.XContentParser;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

import static org.opensearch.action.ValidateActions.addValidationError;

/**
* Request to delete one or more PIT contexts based on IDs.
*/
public class DeletePITRequest extends ActionRequest implements ToXContentObject {

/**
* List of PIT IDs to be deleted , and use "_all" to delete all PIT reader contexts
*/
private List<String> pitIds;

public DeletePITRequest(StreamInput in) throws IOException {
super(in);
pitIds = Arrays.asList(in.readStringArray());
}

public DeletePITRequest(String... pitIds) {
if (pitIds != null) {
this.pitIds = Arrays.asList(pitIds);
}
}

public DeletePITRequest(List<String> pitIds) {
if (pitIds != null) {
this.pitIds = pitIds;
}
}

public DeletePITRequest() {}

public List<String> getPitIds() {
return pitIds;
}

public void setPitIds(List<String> pitIds) {
this.pitIds = pitIds;
}

@Override
public ActionRequestValidationException validate() {
ActionRequestValidationException validationException = null;
if (pitIds == null || pitIds.isEmpty()) {
validationException = addValidationError("no pit ids specified", validationException);
}
return validationException;
}

@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
if (pitIds == null) {
out.writeVInt(0);
} else {
out.writeStringArray(pitIds.toArray(new String[pitIds.size()]));
}
}

public void addPitId(String pitId) {
if (pitIds == null) {
pitIds = new ArrayList<>();
}
pitIds.add(pitId);
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException {
builder.startObject();
builder.startArray("pit_id");
for (String pitId : pitIds) {
builder.value(pitId);
}
builder.endArray();
builder.endObject();
return builder;
}

public void fromXContent(XContentParser parser) throws IOException {
pitIds = null;
if (parser.nextToken() != XContentParser.Token.START_OBJECT) {
throw new IllegalArgumentException("Malformed content, must start with an object");
} else {
XContentParser.Token token;
String currentFieldName = null;
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
currentFieldName = parser.currentName();
} else if ("pit_id".equals(currentFieldName)) {
if (token == XContentParser.Token.START_ARRAY) {
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
if (token.isValue() == false) {
throw new IllegalArgumentException("pit_id array element should only contain pit_id");
}
addPitId(parser.text());
}
} else {
if (token.isValue() == false) {
throw new IllegalArgumentException("pit_id element should only contain pit_id");
}
addPitId(parser.text());
}
} else {
throw new IllegalArgumentException(
"Unknown parameter [" + currentFieldName + "] in request body or parameter is of the wrong type[" + token + "] "
);
}
}
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.action.search;

import org.opensearch.action.ActionResponse;
import org.opensearch.common.ParseField;
import org.opensearch.common.io.stream.StreamInput;
import org.opensearch.common.io.stream.StreamOutput;
import org.opensearch.common.xcontent.ConstructingObjectParser;
import org.opensearch.common.xcontent.ObjectParser;
import org.opensearch.common.xcontent.StatusToXContentObject;
import org.opensearch.common.xcontent.ToXContent;
import org.opensearch.common.xcontent.XContentBuilder;
import org.opensearch.common.xcontent.XContentParser;
import org.opensearch.rest.RestStatus;

import java.io.IOException;

import static org.opensearch.common.xcontent.ConstructingObjectParser.constructorArg;
import static org.opensearch.rest.RestStatus.NOT_FOUND;
import static org.opensearch.rest.RestStatus.OK;

public class DeletePITResponse extends ActionResponse implements StatusToXContentObject {

/**
* This will be true if all PIT reader contexts are deleted.
*/
private final boolean succeeded;

public DeletePITResponse(boolean succeeded) {
this.succeeded = succeeded;
}

public DeletePITResponse(StreamInput in) throws IOException {
super(in);
succeeded = in.readBoolean();
}

/**
* @return Whether the attempt to delete PIT was successful.
*/
public boolean isSucceeded() {
return succeeded;
}

@Override
public RestStatus status() {
return succeeded ? OK : NOT_FOUND;
}

@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeBoolean(succeeded);
}

private static final ParseField SUCCEEDED = new ParseField("succeeded");

private static final ConstructingObjectParser<DeletePITResponse, Void> PARSER = new ConstructingObjectParser<>(
"delete_pit",
true,
a -> new DeletePITResponse((boolean) a[0])
);
static {
PARSER.declareField(constructorArg(), (parser, context) -> parser.booleanValue(), SUCCEEDED, ObjectParser.ValueType.BOOLEAN);
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException {
builder.startObject();
builder.field(SUCCEEDED.getPreferredName(), succeeded);
builder.endObject();
return builder;
}

/**
* Parse the delete PIT response body into a new {@link DeletePITResponse} object
*/
public static DeletePITResponse fromXContent(XContentParser parser) throws IOException {
return PARSER.apply(parser, null);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,10 @@
public class SearchTransportService {

public static final String FREE_CONTEXT_SCROLL_ACTION_NAME = "indices:data/read/search[free_context/scroll]";
public static final String FREE_CONTEXT_PIT_ACTION_NAME = "indices:data/read/search[free_context/pit]";
public static final String FREE_CONTEXT_ACTION_NAME = "indices:data/read/search[free_context]";
public static final String CLEAR_SCROLL_CONTEXTS_ACTION_NAME = "indices:data/read/search[clear_scroll_contexts]";
public static final String DELETE_ALL_PIT_CONTEXTS_ACTION_NAME = "indices:data/read/search[delete_pit_contexts]";
Copy link

Choose a reason for hiding this comment

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

variable name and action name should be consistent for delete all should be consistent with
public static final String FREE_CONTEXT_PIT_ACTION_NAME = "indices:data/read/search[free_context/pit]";

public static final String DFS_ACTION_NAME = "indices:data/read/search[phase/dfs]";
public static final String QUERY_ACTION_NAME = "indices:data/read/search[phase/query]";
public static final String QUERY_ID_ACTION_NAME = "indices:data/read/search[phase/query/id]";
Expand Down Expand Up @@ -142,6 +144,20 @@ public void sendFreeContext(
);
}

public void sendPitFreeContext(
Transport.Connection connection,
ShardSearchContextId contextId,
ActionListener<SearchFreeContextResponse> listener
) {
transportService.sendRequest(
connection,
FREE_CONTEXT_PIT_ACTION_NAME,
new ScrollFreeContextRequest(contextId),
Copy link

Choose a reason for hiding this comment

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

why is this ScrollFreeContextRequest

TransportRequestOptions.EMPTY,
new ActionListenerResponseHandler<>(listener, SearchFreeContextResponse::new)
);
}

public void updatePitContext(
Transport.Connection connection,
UpdatePITContextRequest request,
Expand Down Expand Up @@ -198,6 +214,16 @@ public void sendClearAllScrollContexts(Transport.Connection connection, final Ac
);
}

public void sendDeleteAllPitContexts(Transport.Connection connection, final ActionListener<TransportResponse> listener) {
Copy link

Choose a reason for hiding this comment

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

let's make it uniform and call it freeAllPitContexts maybe

transportService.sendRequest(
connection,
DELETE_ALL_PIT_CONTEXTS_ACTION_NAME,
TransportRequest.Empty.INSTANCE,
TransportRequestOptions.EMPTY,
new ActionListenerResponseHandler<>(listener, (in) -> TransportResponse.Empty.INSTANCE)
Copy link

Choose a reason for hiding this comment

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

why is this returning an empty transport response?

);
}

public void sendExecuteDfs(
Transport.Connection connection,
final ShardSearchRequest request,
Expand Down Expand Up @@ -437,6 +463,18 @@ public static void registerRequestHandler(TransportService transportService, Sea
}
);
TransportActionProxy.registerProxyAction(transportService, FREE_CONTEXT_SCROLL_ACTION_NAME, SearchFreeContextResponse::new);

transportService.registerRequestHandler(
FREE_CONTEXT_PIT_ACTION_NAME,
ThreadPool.Names.SAME,
ScrollFreeContextRequest::new,
(request, channel, task) -> {
boolean freed = searchService.freeReaderContextIfFound(request.id());
channel.sendResponse(new SearchFreeContextResponse(freed));
}
);
TransportActionProxy.registerProxyAction(transportService, FREE_CONTEXT_PIT_ACTION_NAME, SearchFreeContextResponse::new);

transportService.registerRequestHandler(
FREE_CONTEXT_ACTION_NAME,
ThreadPool.Names.SAME,
Expand Down Expand Up @@ -620,6 +658,21 @@ public static void registerRequestHandler(TransportService transportService, Sea
);
TransportActionProxy.registerProxyAction(transportService, UPDATE_READER_CONTEXT_ACTION_NAME, UpdatePitContextResponse::new);

transportService.registerRequestHandler(
DELETE_ALL_PIT_CONTEXTS_ACTION_NAME,
ThreadPool.Names.SAME,
TransportRequest.Empty::new,
(request, channel, task) -> {
searchService.freeAllPitContexts();
channel.sendResponse(TransportResponse.Empty.INSTANCE);
}
);
TransportActionProxy.registerProxyAction(
transportService,
DELETE_ALL_PIT_CONTEXTS_ACTION_NAME,
(in) -> TransportResponse.Empty.INSTANCE
);

}

/**
Expand Down
Loading