-
Notifications
You must be signed in to change notification settings - Fork 0
Caching policy #4
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
base: main
Are you sure you want to change the base?
Changes from 13 commits
fcd7476
87cbe2a
cf18eb9
c876de8
93a16d4
2d41d3e
e6c2fa1
072d788
1c4b4d1
9dd9bf0
0d13d80
ef44275
45b608d
d98b1cc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| /* | ||
| * 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. | ||
| */ | ||
|
|
||
| /* | ||
| * Licensed to Elasticsearch under one or more contributor | ||
| * license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright | ||
| * ownership. Elasticsearch licenses this file to you under | ||
| * the Apache License, Version 2.0 (the "License"); you may | ||
| * not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
|
|
||
| /* | ||
| * Modifications Copyright OpenSearch Contributors. See | ||
| * GitHub history for details. | ||
| */ | ||
|
|
||
| package org.opensearch.indices; | ||
|
|
||
| import org.opensearch.core.common.bytes.BytesReference; | ||
|
|
||
| import java.io.IOException; | ||
|
|
||
| public interface CacheTierPolicy<T> { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where is this generic T being used?
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The idea was to make it explicit what type the BytesReference should be turned into. I could separate out the lines which transform BytesReference -> T as their own function in the interface, as all implementations should need that? |
||
| /** | ||
| * Determines whether this policy allows the data into its cache tier, based on the contents of the BytesReference | ||
| * which can be deserialized into class T. | ||
| * @param data A BytesReference which can be deserialized into class T | ||
| * @return A CheckDataResult object containing whether the data is admitted, and if it isn't, the reason. | ||
| * @throws IOException if the input can't be deserialized to the right class. | ||
| */ | ||
| CheckDataResult checkData(BytesReference data) throws IOException; | ||
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| /* | ||
| * 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. | ||
| */ | ||
|
|
||
| /* | ||
| * Licensed to Elasticsearch under one or more contributor | ||
| * license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright | ||
| * ownership. Elasticsearch licenses this file to you under | ||
| * the Apache License, Version 2.0 (the "License"); you may | ||
| * not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
|
|
||
| /* | ||
| * Modifications Copyright OpenSearch Contributors. See | ||
| * GitHub history for details. | ||
| */ | ||
|
|
||
| package org.opensearch.indices; | ||
|
|
||
| /** | ||
| * A class used by a CacheTierPolicy to return a result for some data input. | ||
| * The results can be chained together in a short-circuiting way. | ||
| */ | ||
| public class CheckDataResult { | ||
| private boolean isAccepted; | ||
| private String deniedReason; // null if the data was accepted, has an explanation if data was rejected | ||
|
||
|
|
||
| public CheckDataResult(boolean isAccepted, String deniedReason) { | ||
| this.isAccepted = isAccepted; | ||
| this.deniedReason = deniedReason; | ||
| } | ||
|
|
||
| public CheckDataResult composeWith(CheckDataResult other) { | ||
| if (!this.isAccepted) { | ||
| return this; | ||
| } else { | ||
| return other; | ||
| } | ||
| } | ||
|
|
||
| public boolean isAccepted() { | ||
| return isAccepted; | ||
| } | ||
|
|
||
| public String getDeniedReason() { | ||
| return deniedReason; | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| /* | ||
| * 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. | ||
| */ | ||
|
|
||
| /* | ||
| * Licensed to Elasticsearch under one or more contributor | ||
| * license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright | ||
| * ownership. Elasticsearch licenses this file to you under | ||
| * the Apache License, Version 2.0 (the "License"); you may | ||
| * not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
|
|
||
| /* | ||
| * Modifications Copyright OpenSearch Contributors. See | ||
| * GitHub history for details. | ||
| */ | ||
|
|
||
| package org.opensearch.indices; | ||
|
|
||
| import org.opensearch.core.common.bytes.BytesReference; | ||
| import org.opensearch.search.query.QuerySearchResult; | ||
|
|
||
| import java.io.IOException; | ||
|
|
||
| /** | ||
| * This policy takes in an array containing an instance of all policies we want to apply to the IRC disk tier. | ||
| * It short circuits these policies' checks together, in the provided order, to get one overall check. | ||
| */ | ||
| public class IndicesRequestCacheDiskTierPolicy implements CacheTierPolicy<QuerySearchResult> { | ||
|
||
| private final CacheTierPolicy<QuerySearchResult>[] policies; | ||
|
||
| private final int numPolicies; | ||
| private final boolean allowedByDefault; | ||
|
||
| public static String DEFAULT_DENIED_REASON = | ||
| "No policies were supplied to IndicesRequestCacheDiskTierPolicy and allowedByDefault = false"; | ||
| // available here for testing purposes | ||
|
|
||
| public IndicesRequestCacheDiskTierPolicy(CacheTierPolicy<QuerySearchResult>[] policies, boolean allowedByDefault) { | ||
| this.policies = policies; | ||
| this.numPolicies = policies.length; | ||
| this.allowedByDefault = allowedByDefault; // default behavior if no other policies are supplied | ||
| } | ||
|
|
||
| @Override | ||
| public CheckDataResult checkData(BytesReference data) throws IOException { | ||
| if (numPolicies == 0) { | ||
| // still need to check the data can be deserialized into QuerySearchResult | ||
| QuerySearchResult qsr; | ||
| try { | ||
| qsr = new QuerySearchResult(data.streamInput()); | ||
| } catch (IllegalStateException ise) { | ||
| throw new IOException(ise); | ||
| } | ||
|
|
||
| if (allowedByDefault) { | ||
| return new CheckDataResult(true, null); | ||
| } else { | ||
| return new CheckDataResult(false, DEFAULT_DENIED_REASON); | ||
| } | ||
| } | ||
| CheckDataResult result = policies[0].checkData(data); | ||
| if (numPolicies > 1) { | ||
| for (int i = 1; i < numPolicies; i++) { | ||
| result = result.composeWith(policies[i].checkData(data)); | ||
| if (!result.isAccepted()) { | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| return result; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| /* | ||
| * 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. | ||
| */ | ||
|
|
||
| /* | ||
| * Licensed to Elasticsearch under one or more contributor | ||
| * license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright | ||
| * ownership. Elasticsearch licenses this file to you under | ||
| * the Apache License, Version 2.0 (the "License"); you may | ||
| * not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
|
|
||
| /* | ||
| * Modifications Copyright OpenSearch Contributors. See | ||
| * GitHub history for details. | ||
| */ | ||
|
|
||
| package org.opensearch.indices; | ||
|
|
||
| import org.opensearch.common.settings.ClusterSettings; | ||
| import org.opensearch.common.settings.Setting; | ||
| import org.opensearch.common.settings.Settings; | ||
| import org.opensearch.common.unit.TimeValue; | ||
| import org.opensearch.core.common.bytes.BytesReference; | ||
| import org.opensearch.search.query.QuerySearchResult; | ||
|
|
||
| import java.io.IOException; | ||
|
|
||
| /** | ||
| * A cache tier policy which accepts queries whose took time is greater than some threshold, | ||
| * which is specified as a dynamic cluster-level setting. The threshold should be set to approximately | ||
| * the time it takes to get a result from the cache tier. | ||
| */ | ||
| public class IndicesRequestCacheTookTimePolicy implements CacheTierPolicy<QuerySearchResult> { | ||
|
||
| public static final Setting<TimeValue> INDICES_REQUEST_CACHE_DISK_TOOKTIME_THRESHOLD_SETTING = Setting.positiveTimeSetting( | ||
| "index.requests.cache.disk.tooktime.threshold", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As we might rename this class to generic DiskTierTookTimePolicy, you will have to take setting prefix "indices.request.cache" as a parameter in constructor. It is done in a similar way in framework PR. |
||
| new TimeValue(10), | ||
| Setting.Property.Dynamic, | ||
| Setting.Property.NodeScope | ||
| ); | ||
|
|
||
| private TimeValue threshold; | ||
|
|
||
| public IndicesRequestCacheTookTimePolicy(Settings settings, ClusterSettings clusterSettings) { | ||
| this.threshold = INDICES_REQUEST_CACHE_DISK_TOOKTIME_THRESHOLD_SETTING.get(settings); | ||
| clusterSettings.addSettingsUpdateConsumer(INDICES_REQUEST_CACHE_DISK_TOOKTIME_THRESHOLD_SETTING, this::setThreshold); | ||
| } | ||
|
|
||
| public void setThreshold(TimeValue threshold) { // public so that we can manually set value in unit test | ||
|
||
| this.threshold = threshold; | ||
| } | ||
|
|
||
| protected String buildDeniedString(TimeValue tookTime, TimeValue threshold) { | ||
| // separating out for use in testing | ||
| return "Query took time " + tookTime.getMillis() + " ms is less than threshold value " + threshold.getMillis() + " ms"; | ||
| } | ||
|
|
||
| @Override | ||
| public CheckDataResult checkData(BytesReference data) throws IOException { | ||
| QuerySearchResult qsr; | ||
| try { | ||
| qsr = new QuerySearchResult(data.streamInput()); | ||
| } catch (IllegalStateException ise) { | ||
| throw new IOException(ise); | ||
| } | ||
| TimeValue tookTime = TimeValue.timeValueNanos(qsr.getTookTimeNanos()); | ||
| boolean isAccepted = true; | ||
| String deniedReason = null; | ||
| if (tookTime.compareTo(threshold) < 0) { // negative -> tookTime is shorter than threshold | ||
| isAccepted = false; | ||
| deniedReason = buildDeniedString(tookTime, threshold); | ||
| } | ||
| return new CheckDataResult(isAccepted, deniedReason); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move these under suggested common/cache/tier package