From 83590e1340e8b093605b4706ad880b333786819c Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Mon, 9 Oct 2023 17:01:34 -0700 Subject: [PATCH 01/22] initial commit --- .../main/resources/fips_java_bcjsse_11.policy | 6 + .../crypto-kms/licenses/slf4j-api-LICENSE.txt | 21 -- .../licenses/slf4j-api-1.7.36.jar.sha1 | 1 - .../licenses/slf4j-api-LICENSE.txt | 21 -- .../licenses/slf4j-api-NOTICE.txt | 0 .../licenses/slf4j-api-1.7.36.jar.sha1 | 1 - .../licenses/slf4j-api-NOTICE.txt | 0 .../licenses/slf4j-api-1.7.36.jar.sha1 | 1 - .../licenses/slf4j-api-1.7.36.jar.sha1 | 1 - .../licenses/slf4j-api-1.7.36.jar.sha1 | 1 - .../licenses/slf4j-api-1.7.36.jar.sha1 | 1 - server/build.gradle | 4 + server/licenses/ehcache-3.10.8.jar.sha1 | 1 + server/licenses/ehcache-LICENSE.txt | 201 ++++++++++++++++++ server/licenses/ehcache-NOTICE.txt | 5 + .../licenses/slf4j-api-1.7.36.jar.sha1 | 0 .../licenses/slf4j-api-LICENSE.txt | 4 +- .../licenses/slf4j-api-NOTICE.txt | 0 .../indices/EhcacheDiskCachingTier.java | 113 ++++++++++ .../indices/IndicesRequestCache.java | 5 +- 20 files changed, 337 insertions(+), 50 deletions(-) delete mode 100644 plugins/crypto-kms/licenses/slf4j-api-LICENSE.txt delete mode 100644 plugins/discovery-ec2/licenses/slf4j-api-1.7.36.jar.sha1 delete mode 100644 plugins/discovery-ec2/licenses/slf4j-api-LICENSE.txt delete mode 100644 plugins/discovery-ec2/licenses/slf4j-api-NOTICE.txt delete mode 100644 plugins/identity-shiro/licenses/slf4j-api-1.7.36.jar.sha1 delete mode 100644 plugins/identity-shiro/licenses/slf4j-api-NOTICE.txt delete mode 100644 plugins/ingest-attachment/licenses/slf4j-api-1.7.36.jar.sha1 delete mode 100644 plugins/repository-azure/licenses/slf4j-api-1.7.36.jar.sha1 delete mode 100644 plugins/repository-hdfs/licenses/slf4j-api-1.7.36.jar.sha1 delete mode 100644 plugins/repository-s3/licenses/slf4j-api-1.7.36.jar.sha1 create mode 100644 server/licenses/ehcache-3.10.8.jar.sha1 create mode 100644 server/licenses/ehcache-LICENSE.txt create mode 100644 server/licenses/ehcache-NOTICE.txt rename {plugins/crypto-kms => server}/licenses/slf4j-api-1.7.36.jar.sha1 (100%) rename {plugins/identity-shiro => server}/licenses/slf4j-api-LICENSE.txt (95%) rename {plugins/crypto-kms => server}/licenses/slf4j-api-NOTICE.txt (100%) create mode 100644 server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java diff --git a/buildSrc/src/main/resources/fips_java_bcjsse_11.policy b/buildSrc/src/main/resources/fips_java_bcjsse_11.policy index 10193f4eb385d..4c4b9dc670a65 100644 --- a/buildSrc/src/main/resources/fips_java_bcjsse_11.policy +++ b/buildSrc/src/main/resources/fips_java_bcjsse_11.policy @@ -26,4 +26,10 @@ grant { permission org.bouncycastle.crypto.CryptoServicesPermission "exportSecretKey"; permission org.bouncycastle.crypto.CryptoServicesPermission "exportPrivateKey"; permission java.io.FilePermission "${javax.net.ssl.trustStore}", "read"; + //permission java.lang.RuntimePermission "createClassLoader"; // needed to create an ehcache disk tier cache +}; + +grant codeBase "${codebase.ehcache}" { + // needed to allow ehcache to create the classloader + permission java.security.AllPermission; }; diff --git a/plugins/crypto-kms/licenses/slf4j-api-LICENSE.txt b/plugins/crypto-kms/licenses/slf4j-api-LICENSE.txt deleted file mode 100644 index 2be7689435062..0000000000000 --- a/plugins/crypto-kms/licenses/slf4j-api-LICENSE.txt +++ /dev/null @@ -1,21 +0,0 @@ -Copyright (c) 2004-2022 QOS.ch -All rights reserved. - -Permission is hereby granted, free of charge, to any person obtaining -a copy of this software and associated documentation files (the -"Software"), to deal in the Software without restriction, including -without limitation the rights to use, copy, modify, merge, publish, -distribute, sublicense, and/or sell copies of the Software, and to -permit persons to whom the Software is furnished to do so, subject to -the following conditions: - -The above copyright notice and this permission notice shall be -included in all copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, -EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND -NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE -LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION -OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION -WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. \ No newline at end of file diff --git a/plugins/discovery-ec2/licenses/slf4j-api-1.7.36.jar.sha1 b/plugins/discovery-ec2/licenses/slf4j-api-1.7.36.jar.sha1 deleted file mode 100644 index 77b9917528382..0000000000000 --- a/plugins/discovery-ec2/licenses/slf4j-api-1.7.36.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -6c62681a2f655b49963a5983b8b0950a6120ae14 \ No newline at end of file diff --git a/plugins/discovery-ec2/licenses/slf4j-api-LICENSE.txt b/plugins/discovery-ec2/licenses/slf4j-api-LICENSE.txt deleted file mode 100644 index 2be7689435062..0000000000000 --- a/plugins/discovery-ec2/licenses/slf4j-api-LICENSE.txt +++ /dev/null @@ -1,21 +0,0 @@ -Copyright (c) 2004-2022 QOS.ch -All rights reserved. - -Permission is hereby granted, free of charge, to any person obtaining -a copy of this software and associated documentation files (the -"Software"), to deal in the Software without restriction, including -without limitation the rights to use, copy, modify, merge, publish, -distribute, sublicense, and/or sell copies of the Software, and to -permit persons to whom the Software is furnished to do so, subject to -the following conditions: - -The above copyright notice and this permission notice shall be -included in all copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, -EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND -NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE -LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION -OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION -WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. \ No newline at end of file diff --git a/plugins/discovery-ec2/licenses/slf4j-api-NOTICE.txt b/plugins/discovery-ec2/licenses/slf4j-api-NOTICE.txt deleted file mode 100644 index e69de29bb2d1d..0000000000000 diff --git a/plugins/identity-shiro/licenses/slf4j-api-1.7.36.jar.sha1 b/plugins/identity-shiro/licenses/slf4j-api-1.7.36.jar.sha1 deleted file mode 100644 index 77b9917528382..0000000000000 --- a/plugins/identity-shiro/licenses/slf4j-api-1.7.36.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -6c62681a2f655b49963a5983b8b0950a6120ae14 \ No newline at end of file diff --git a/plugins/identity-shiro/licenses/slf4j-api-NOTICE.txt b/plugins/identity-shiro/licenses/slf4j-api-NOTICE.txt deleted file mode 100644 index e69de29bb2d1d..0000000000000 diff --git a/plugins/ingest-attachment/licenses/slf4j-api-1.7.36.jar.sha1 b/plugins/ingest-attachment/licenses/slf4j-api-1.7.36.jar.sha1 deleted file mode 100644 index 77b9917528382..0000000000000 --- a/plugins/ingest-attachment/licenses/slf4j-api-1.7.36.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -6c62681a2f655b49963a5983b8b0950a6120ae14 \ No newline at end of file diff --git a/plugins/repository-azure/licenses/slf4j-api-1.7.36.jar.sha1 b/plugins/repository-azure/licenses/slf4j-api-1.7.36.jar.sha1 deleted file mode 100644 index 77b9917528382..0000000000000 --- a/plugins/repository-azure/licenses/slf4j-api-1.7.36.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -6c62681a2f655b49963a5983b8b0950a6120ae14 \ No newline at end of file diff --git a/plugins/repository-hdfs/licenses/slf4j-api-1.7.36.jar.sha1 b/plugins/repository-hdfs/licenses/slf4j-api-1.7.36.jar.sha1 deleted file mode 100644 index 77b9917528382..0000000000000 --- a/plugins/repository-hdfs/licenses/slf4j-api-1.7.36.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -6c62681a2f655b49963a5983b8b0950a6120ae14 \ No newline at end of file diff --git a/plugins/repository-s3/licenses/slf4j-api-1.7.36.jar.sha1 b/plugins/repository-s3/licenses/slf4j-api-1.7.36.jar.sha1 deleted file mode 100644 index 77b9917528382..0000000000000 --- a/plugins/repository-s3/licenses/slf4j-api-1.7.36.jar.sha1 +++ /dev/null @@ -1 +0,0 @@ -6c62681a2f655b49963a5983b8b0950a6120ae14 \ No newline at end of file diff --git a/server/build.gradle b/server/build.gradle index f6db3d53a0dcc..77cc411677ac5 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -158,6 +158,10 @@ dependencies { api "com.google.protobuf:protobuf-java:${versions.protobuf}" api "jakarta.annotation:jakarta.annotation-api:${versions.jakarta_annotation}" + // ehcache + api "org.ehcache:ehcache:3.10.8" + api "org.slf4j:slf4j-api:1.7.36" + testImplementation(project(":test:framework")) { // tests use the locally compiled version of server exclude group: 'org.opensearch', module: 'server' diff --git a/server/licenses/ehcache-3.10.8.jar.sha1 b/server/licenses/ehcache-3.10.8.jar.sha1 new file mode 100644 index 0000000000000..dee07e9238ebf --- /dev/null +++ b/server/licenses/ehcache-3.10.8.jar.sha1 @@ -0,0 +1 @@ +f0d50ede46609db78413ca7f4250d348a597b101 \ No newline at end of file diff --git a/server/licenses/ehcache-LICENSE.txt b/server/licenses/ehcache-LICENSE.txt new file mode 100644 index 0000000000000..8dada3edaf50d --- /dev/null +++ b/server/licenses/ehcache-LICENSE.txt @@ -0,0 +1,201 @@ + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "{}" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright {yyyy} {name of copyright owner} + + Licensed 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. diff --git a/server/licenses/ehcache-NOTICE.txt b/server/licenses/ehcache-NOTICE.txt new file mode 100644 index 0000000000000..1dbd38242cc98 --- /dev/null +++ b/server/licenses/ehcache-NOTICE.txt @@ -0,0 +1,5 @@ +Ehcache V3 +Copyright 2014-2023 Terracotta, Inc. + +The product includes software from the Apache Commons Lang project, +under the Apache License 2.0 (see: org.ehcache.impl.internal.classes.commonslang) diff --git a/plugins/crypto-kms/licenses/slf4j-api-1.7.36.jar.sha1 b/server/licenses/slf4j-api-1.7.36.jar.sha1 similarity index 100% rename from plugins/crypto-kms/licenses/slf4j-api-1.7.36.jar.sha1 rename to server/licenses/slf4j-api-1.7.36.jar.sha1 diff --git a/plugins/identity-shiro/licenses/slf4j-api-LICENSE.txt b/server/licenses/slf4j-api-LICENSE.txt similarity index 95% rename from plugins/identity-shiro/licenses/slf4j-api-LICENSE.txt rename to server/licenses/slf4j-api-LICENSE.txt index 8fda22f4d72f6..a51675a21c10f 100644 --- a/plugins/identity-shiro/licenses/slf4j-api-LICENSE.txt +++ b/server/licenses/slf4j-api-LICENSE.txt @@ -1,4 +1,4 @@ -Copyright (c) 2004-2014 QOS.ch +Copyright (c) 2004-2022 QOS.ch Sarl (Switzerland) All rights reserved. Permission is hereby granted, free of charge, to any person obtaining @@ -19,3 +19,5 @@ NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + + diff --git a/plugins/crypto-kms/licenses/slf4j-api-NOTICE.txt b/server/licenses/slf4j-api-NOTICE.txt similarity index 100% rename from plugins/crypto-kms/licenses/slf4j-api-NOTICE.txt rename to server/licenses/slf4j-api-NOTICE.txt diff --git a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java new file mode 100644 index 0000000000000..960b5f76f9ac7 --- /dev/null +++ b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java @@ -0,0 +1,113 @@ +/* + * 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.indices; + +import org.ehcache.config.builders.CacheConfigurationBuilder; +import org.ehcache.config.builders.CacheManagerBuilder; +import org.ehcache.config.builders.ResourcePoolsBuilder; +import org.ehcache.config.units.MemoryUnit; +import org.opensearch.common.cache.RemovalListener; +import org.ehcache.Cache; +import org.ehcache.CacheManager; + +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Collections; + +public class EhcacheDiskCachingTier implements CachingTier { + + private CacheManager cacheManager; + private Cache cache; + private final Class keyType; // I think these are needed to pass to newCacheConfigurationBuilder + private final Class valueType; + //private final Path DISK_CACHE_FP = Paths.get("disk_cache_tier"); // ? + private final String DISK_CACHE_FP = "disk_cache_tier"; + // private RBMIntKeyLookupStore keystore; + // private CacheTierPolicy[] policies; + // private IndicesRequestCacheDiskTierPolicy policy; + private RemovalListener removalListener = notification -> {}; // based on on-heap + + public EhcacheDiskCachingTier(long maxWeightInBytes, long maxKeystoreWeightInBytes, Class keyType, Class valueType) { + this.keyType = keyType; + this.valueType = valueType; + String cacheAlias = "diskTier"; + /*try { + Path file = Files.createFile(DISK_CACHE_FP); + } catch (Exception ignored) {}*/ + this.cacheManager = CacheManagerBuilder.newCacheManagerBuilder() + //.with(CacheManagerBuilder.persistence(new File(DISK_CACHE_FP))) + .with(CacheManagerBuilder.persistence(DISK_CACHE_FP)) + .withCache(cacheAlias, CacheConfigurationBuilder.newCacheConfigurationBuilder( + keyType, valueType, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(maxWeightInBytes, MemoryUnit.B, true) + )).build(true); + /*this.cacheManager = CacheManagerBuilder.newCacheManagerBuilder() + .with(CacheManagerBuilder.persistence(new File(DISK_CACHE_FP))) + .withCache(cacheAlias, CacheConfigurationBuilder.newCacheConfigurationBuilder( + IndicesRequestCache.Key.class, BytesReference.class, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(maxWeightInBytes, MemoryUnit.B, true) + )).build(true);*/ + this.cache = cacheManager.getCache(cacheAlias, keyType, valueType); + // this.keystore = new RBMIntKeyLookupStore((int) Math.pow(2, 28), maxKeystoreWeightInBytes); + // this.policies = new CacheTierPolicy[]{ new IndicesRequestCacheTookTimePolicy(settings, clusterSettings) }; + // this.policy = new IndicesRequestCacheDiskTierPolicy(this.policies, true); + } + + @Override + public V get(K key) { + // keystore check goes here + // if (keystore.contains(key.hashCode()) + // do we need to do the future stuff? + return cache.get(key); // do we want to somehow keep track of how long each cache.get takes? + //return null; + } + + @Override + public void put(K key, V value) { + // CheckDataResult policyResult = policy.checkData(value) + // if (policyResult.isAccepted()) { + cache.put(key, value); + // do we need to check for replacement, as onheap does? + // else { do something with policyResult.deniedReason()? } + } + + @Override + public V computeIfAbsent(K key, TieredCacheLoader loader) throws Exception { + return null; + } + + @Override + public void invalidate(K key) {} + + @Override + public V compute(K key, TieredCacheLoader loader) throws Exception { + return null; + } + + @Override + public void setRemovalListener(RemovalListener removalListener) {} + + @Override + public void invalidateAll() {} + + @Override + public Iterable keys() { + return Collections::emptyIterator; + } + + @Override + public int count() { + return 0; + } + + @Override + public TierType getTierType() { + return TierType.DISK; + } +} diff --git a/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java b/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java index ab80e63a4abce..c43e09992be20 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java +++ b/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java @@ -124,9 +124,12 @@ public final class IndicesRequestCache implements TieredCacheEventListener k.ramBytesUsed() + v.ramBytesUsed() ).setMaximumWeight(sizeInBytes).setExpireAfterAccess(expire).build(); + int diskTierWeight = 100 * 1048576; // 100 MB, for testing only + EhcacheDiskCachingTier diskCachingTier; + diskCachingTier = new EhcacheDiskCachingTier<>(diskTierWeight, 0, Key.class, BytesReference.class); tieredCacheHandler = new TieredCacheSpilloverStrategyHandler.Builder().setOnHeapCachingTier( openSearchOnHeapCache - ).setOnDiskCachingTier(new DummyDiskCachingTier<>()).setTieredCacheEventListener(this).build(); + ).setOnDiskCachingTier(diskCachingTier).setTieredCacheEventListener(this).build(); } @Override From aa0a5d7b555209de3384e309b13a114ac157e462 Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Tue, 10 Oct 2023 10:24:28 -0700 Subject: [PATCH 02/22] fixed permissions --- buildSrc/src/main/resources/fips_java_bcjsse_11.policy | 6 ------ server/disk_cache_tier/.lock | 0 .../org/opensearch/indices/EhcacheDiskCachingTier.java | 10 ---------- .../resources/org/opensearch/bootstrap/security.policy | 8 ++++++++ 4 files changed, 8 insertions(+), 16 deletions(-) create mode 100644 server/disk_cache_tier/.lock diff --git a/buildSrc/src/main/resources/fips_java_bcjsse_11.policy b/buildSrc/src/main/resources/fips_java_bcjsse_11.policy index 4c4b9dc670a65..10193f4eb385d 100644 --- a/buildSrc/src/main/resources/fips_java_bcjsse_11.policy +++ b/buildSrc/src/main/resources/fips_java_bcjsse_11.policy @@ -26,10 +26,4 @@ grant { permission org.bouncycastle.crypto.CryptoServicesPermission "exportSecretKey"; permission org.bouncycastle.crypto.CryptoServicesPermission "exportPrivateKey"; permission java.io.FilePermission "${javax.net.ssl.trustStore}", "read"; - //permission java.lang.RuntimePermission "createClassLoader"; // needed to create an ehcache disk tier cache -}; - -grant codeBase "${codebase.ehcache}" { - // needed to allow ehcache to create the classloader - permission java.security.AllPermission; }; diff --git a/server/disk_cache_tier/.lock b/server/disk_cache_tier/.lock new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java index 960b5f76f9ac7..fdbc38fe6a716 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java @@ -28,7 +28,6 @@ public class EhcacheDiskCachingTier implements CachingTier { private Cache cache; private final Class keyType; // I think these are needed to pass to newCacheConfigurationBuilder private final Class valueType; - //private final Path DISK_CACHE_FP = Paths.get("disk_cache_tier"); // ? private final String DISK_CACHE_FP = "disk_cache_tier"; // private RBMIntKeyLookupStore keystore; // private CacheTierPolicy[] policies; @@ -39,20 +38,11 @@ public EhcacheDiskCachingTier(long maxWeightInBytes, long maxKeystoreWeightInByt this.keyType = keyType; this.valueType = valueType; String cacheAlias = "diskTier"; - /*try { - Path file = Files.createFile(DISK_CACHE_FP); - } catch (Exception ignored) {}*/ this.cacheManager = CacheManagerBuilder.newCacheManagerBuilder() - //.with(CacheManagerBuilder.persistence(new File(DISK_CACHE_FP))) .with(CacheManagerBuilder.persistence(DISK_CACHE_FP)) .withCache(cacheAlias, CacheConfigurationBuilder.newCacheConfigurationBuilder( keyType, valueType, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(maxWeightInBytes, MemoryUnit.B, true) )).build(true); - /*this.cacheManager = CacheManagerBuilder.newCacheManagerBuilder() - .with(CacheManagerBuilder.persistence(new File(DISK_CACHE_FP))) - .withCache(cacheAlias, CacheConfigurationBuilder.newCacheConfigurationBuilder( - IndicesRequestCache.Key.class, BytesReference.class, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(maxWeightInBytes, MemoryUnit.B, true) - )).build(true);*/ this.cache = cacheManager.getCache(cacheAlias, keyType, valueType); // this.keystore = new RBMIntKeyLookupStore((int) Math.pow(2, 28), maxKeystoreWeightInBytes); // this.policies = new CacheTierPolicy[]{ new IndicesRequestCacheTookTimePolicy(settings, clusterSettings) }; diff --git a/server/src/main/resources/org/opensearch/bootstrap/security.policy b/server/src/main/resources/org/opensearch/bootstrap/security.policy index 77cd0ab05278e..20e5671d582a6 100644 --- a/server/src/main/resources/org/opensearch/bootstrap/security.policy +++ b/server/src/main/resources/org/opensearch/bootstrap/security.policy @@ -188,4 +188,12 @@ grant { permission java.io.FilePermission "/sys/fs/cgroup/memory", "read"; permission java.io.FilePermission "/sys/fs/cgroup/memory/-", "read"; + // ehcache + permission java.lang.RuntimePermission "createClassLoader"; + permission java.lang.RuntimePermission "accessClassInPackage.sun.misc"; + permission java.lang.RuntimePermission "getenv.*"; + permission java.io.FilePermission "disk_cache_tier", "read"; // change this to wherever we will put disk tier folder + permission java.io.FilePermission "disk_cache_tier", "write"; + permission java.io.FilePermission "disk_cache_tier/-", "read"; + permission java.io.FilePermission "disk_cache_tier/-", "write"; }; From 41dc035643f0962a3a8e58338ebba3dd498ae7a7 Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Tue, 10 Oct 2023 16:26:52 -0700 Subject: [PATCH 03/22] Set up event listener to create RemovalNotification objects, also misc functions --- .../core/common/bytes/BytesReference.java | 3 +- .../ehcache-disk-store.data | Bin 0 -> 16384 bytes .../ehcache-disk-store.meta | 4 + .../indices/EhcacheDiskCachingTier.java | 121 +++++++++++++++--- .../indices/IndicesRequestCache.java | 6 +- 5 files changed, 113 insertions(+), 21 deletions(-) create mode 100644 server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.data create mode 100644 server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.meta diff --git a/libs/core/src/main/java/org/opensearch/core/common/bytes/BytesReference.java b/libs/core/src/main/java/org/opensearch/core/common/bytes/BytesReference.java index 9d24d3653397b..60613d4e2a997 100644 --- a/libs/core/src/main/java/org/opensearch/core/common/bytes/BytesReference.java +++ b/libs/core/src/main/java/org/opensearch/core/common/bytes/BytesReference.java @@ -45,6 +45,7 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; +import java.io.Serializable; import java.nio.ByteBuffer; import java.util.ArrayList; @@ -54,7 +55,7 @@ * @opensearch.api */ @PublicApi(since = "1.0.0") -public interface BytesReference extends Comparable, ToXContentFragment { +public interface BytesReference extends Comparable, ToXContentFragment, Serializable { // another lie! /** * Convert an {@link XContentBuilder} into a BytesReference. This method closes the builder, diff --git a/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.data b/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.data new file mode 100644 index 0000000000000000000000000000000000000000..294f4016d05bdd696670c4840f1f36a71f9239de GIT binary patch literal 16384 zcmeIuF#!Mo0K%a4Pi+hzh(KY$fB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM q7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjMyaxtA00031 literal 0 HcmV?d00001 diff --git a/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.meta b/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.meta new file mode 100644 index 0000000000000..2d3e90d195f10 --- /dev/null +++ b/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.meta @@ -0,0 +1,4 @@ +#Key and value types +#Tue Oct 10 19:13:45 EDT 2023 +valueType=org.opensearch.core.common.bytes.BytesReference +keyType=org.opensearch.indices.IndicesRequestCache$Key diff --git a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java index fdbc38fe6a716..42445bc3ea6ef 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java @@ -9,41 +9,65 @@ package org.opensearch.indices; import org.ehcache.config.builders.CacheConfigurationBuilder; +import org.ehcache.config.builders.CacheEventListenerConfigurationBuilder; import org.ehcache.config.builders.CacheManagerBuilder; import org.ehcache.config.builders.ResourcePoolsBuilder; import org.ehcache.config.units.MemoryUnit; +import org.ehcache.core.internal.statistics.DefaultStatisticsService; +import org.ehcache.core.spi.service.StatisticsService; +import org.ehcache.core.statistics.CacheStatistics; +import org.ehcache.event.CacheEvent; +import org.ehcache.event.CacheEventListener; +import org.ehcache.event.EventType; import org.opensearch.common.cache.RemovalListener; import org.ehcache.Cache; import org.ehcache.CacheManager; +import org.opensearch.common.cache.RemovalNotification; +import org.opensearch.common.cache.RemovalReason; -import java.io.File; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; import java.util.Collections; -public class EhcacheDiskCachingTier implements CachingTier { +public class EhcacheDiskCachingTier implements CachingTier, RemovalListener { private CacheManager cacheManager; private Cache cache; + private final Class keyType; // I think these are needed to pass to newCacheConfigurationBuilder private final Class valueType; private final String DISK_CACHE_FP = "disk_cache_tier"; + private RemovalListener removalListener; + private final StatisticsService statsService; + private final CacheStatistics cacheStats; // private RBMIntKeyLookupStore keystore; // private CacheTierPolicy[] policies; // private IndicesRequestCacheDiskTierPolicy policy; - private RemovalListener removalListener = notification -> {}; // based on on-heap + //private RemovalListener removalListener = notification -> {}; // based on on-heap public EhcacheDiskCachingTier(long maxWeightInBytes, long maxKeystoreWeightInBytes, Class keyType, Class valueType) { this.keyType = keyType; this.valueType = valueType; String cacheAlias = "diskTier"; + this.statsService = new DefaultStatisticsService(); + + // our EhcacheEventListener should receive events every time an entry is removed for some reason, but not when it's created + CacheEventListenerConfigurationBuilder listenerConfig = CacheEventListenerConfigurationBuilder + .newEventListenerConfiguration(new EhcacheEventListener(this), + EventType.EVICTED, + EventType.EXPIRED, + EventType.REMOVED, + EventType.UPDATED) + .ordered().asynchronous(); // ordered() has some performance penalty as compared to unordered(), we can also use synchronous() + this.cacheManager = CacheManagerBuilder.newCacheManagerBuilder() + .using(statsService) .with(CacheManagerBuilder.persistence(DISK_CACHE_FP)) .withCache(cacheAlias, CacheConfigurationBuilder.newCacheConfigurationBuilder( - keyType, valueType, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(maxWeightInBytes, MemoryUnit.B, true) - )).build(true); + keyType, valueType, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(maxWeightInBytes, MemoryUnit.B, true)) + .withService(listenerConfig) + ).build(true); this.cache = cacheManager.getCache(cacheAlias, keyType, valueType); + this.cacheStats = statsService.getCacheStatistics(cacheAlias); + // this.keystore = new RBMIntKeyLookupStore((int) Math.pow(2, 28), maxKeystoreWeightInBytes); // this.policies = new CacheTierPolicy[]{ new IndicesRequestCacheTookTimePolicy(settings, clusterSettings) }; // this.policy = new IndicesRequestCacheDiskTierPolicy(this.policies, true); @@ -51,53 +75,112 @@ public EhcacheDiskCachingTier(long maxWeightInBytes, long maxKeystoreWeightInByt @Override public V get(K key) { - // keystore check goes here - // if (keystore.contains(key.hashCode()) - // do we need to do the future stuff? + // do we need to do the future stuff? I don't think so? + + // if (keystore.contains(key.hashCode()) { return cache.get(key); // do we want to somehow keep track of how long each cache.get takes? - //return null; + // } + // return null; + } @Override public void put(K key, V value) { + // No need to get old value, this is handled by EhcacheEventListener. + // CheckDataResult policyResult = policy.checkData(value) // if (policyResult.isAccepted()) { cache.put(key, value); - // do we need to check for replacement, as onheap does? // else { do something with policyResult.deniedReason()? } + // } } @Override public V computeIfAbsent(K key, TieredCacheLoader loader) throws Exception { - return null; + return null; // should not need to fill out, Cache.computeIfAbsent is always used } @Override - public void invalidate(K key) {} + public void invalidate(K key) { + // keep keystore check to avoid unneeded disk seek + // RemovalNotification is handled by EhcacheEventListener + + // if (keystore.contains(key.hashCode()) { + cache.remove(key); + // } + } @Override public V compute(K key, TieredCacheLoader loader) throws Exception { - return null; + return null; // should not need to fill out, Cache.compute is always used } @Override - public void setRemovalListener(RemovalListener removalListener) {} + public void setRemovalListener(RemovalListener removalListener) { + this.removalListener = removalListener; // this is passed the spillover strategy, same as on-heap + } @Override - public void invalidateAll() {} + public void invalidateAll() { + // can we just wipe the cache and start over? Or do we need to create a bunch of RemovalNotifications? + } @Override public Iterable keys() { + // ehcache doesn't provide a method like this, because it might be a huge number of keys that consume all + // the memory. Do we want this method for disk tier? return Collections::emptyIterator; } @Override public int count() { - return 0; + // this might be an expensive disk-seek call. Might be better to keep track ourselves? + return (int) cacheStats.getTierStatistics().get("Disk").getMappings(); } @Override public TierType getTierType() { return TierType.DISK; } + + @Override + public void onRemoval(RemovalNotification notification) { + removalListener.onRemoval(notification); + } + + // See https://stackoverflow.com/questions/45827753/listenerobject-not-found-in-imports-for-ehcache-3 for API reference + // This class is used to get the old value from mutating calls to the cache and use those to create a RemovalNotification + private class EhcacheEventListener implements CacheEventListener { + private RemovalListener removalListener; + EhcacheEventListener(RemovalListener removalListener) { + this.removalListener = removalListener; + } + @Override + public void onEvent(CacheEvent event) { + // send a RemovalNotification + K key = (K) event.getKey(); // I think these casts should be ok? + V oldValue = (V) event.getOldValue(); + V newValue = (V) event.getNewValue(); + EventType eventType = event.getType(); + RemovalReason reason; + switch (eventType) { + case EVICTED: + reason = RemovalReason.EVICTED; // why is there both RemovalReason.EVICTED and RemovalReason.CAPACITY? + break; + case EXPIRED: + reason = RemovalReason.INVALIDATED; // not sure about this one + break; + case REMOVED: + reason = RemovalReason.INVALIDATED; // we use cache.remove() to invalidate keys, but this might overlap with RemovalReason.EXPLICIT? + break; + case UPDATED: + reason = RemovalReason.REPLACED; + break; + default: + reason = null; + } + // we don't subscribe to CREATED type, which is the only other option + removalListener.onRemoval(new RemovalNotification(key, oldValue, reason)); + } + } } diff --git a/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java b/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java index c43e09992be20..d55c188b8a418 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java +++ b/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java @@ -46,11 +46,13 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.concurrent.ConcurrentCollections; +import org.opensearch.core.common.bytes.BytesArray; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.common.unit.ByteSizeValue; import java.io.Closeable; import java.io.IOException; +import java.io.Serializable; import java.util.Collection; import java.util.Collections; import java.util.HashSet; @@ -125,6 +127,8 @@ public final class IndicesRequestCache implements TieredCacheEventListener diskCachingTier; diskCachingTier = new EhcacheDiskCachingTier<>(diskTierWeight, 0, Key.class, BytesReference.class); tieredCacheHandler = new TieredCacheSpilloverStrategyHandler.Builder().setOnHeapCachingTier( @@ -273,7 +277,7 @@ interface CacheEntity extends Accountable { * * @opensearch.internal */ - static class Key implements Accountable { + static class Key implements Accountable, Serializable { // lies! it doesnt implement serializable in a functioning way, but for testing private static final long BASE_RAM_BYTES_USED = RamUsageEstimator.shallowSizeOfInstance(Key.class); public final CacheEntity entity; // use as identity equality From 4ba49afbca3f26a6b42a297ea2a8094841dee7ed Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Wed, 11 Oct 2023 10:54:57 -0700 Subject: [PATCH 04/22] Added misc getter methods --- .../indices/EhcacheDiskCachingTier.java | 62 ++++++++++++++++--- 1 file changed, 53 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java index 42445bc3ea6ef..3f9057c436be0 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java @@ -16,9 +16,11 @@ import org.ehcache.core.internal.statistics.DefaultStatisticsService; import org.ehcache.core.spi.service.StatisticsService; import org.ehcache.core.statistics.CacheStatistics; +import org.ehcache.core.statistics.TierStatistics; import org.ehcache.event.CacheEvent; import org.ehcache.event.CacheEventListener; import org.ehcache.event.EventType; +import org.opensearch.common.ExponentiallyWeightedMovingAverage; import org.opensearch.common.cache.RemovalListener; import org.ehcache.Cache; import org.ehcache.CacheManager; @@ -26,18 +28,21 @@ import org.opensearch.common.cache.RemovalReason; import java.util.Collections; +import java.util.Map; public class EhcacheDiskCachingTier implements CachingTier, RemovalListener { private CacheManager cacheManager; - private Cache cache; + private final Cache cache; private final Class keyType; // I think these are needed to pass to newCacheConfigurationBuilder private final Class valueType; - private final String DISK_CACHE_FP = "disk_cache_tier"; + private final String DISK_CACHE_FP = "disk_cache_tier"; // this should probably be defined somewhere else since we need to change security.policy based on its value private RemovalListener removalListener; private final StatisticsService statsService; private final CacheStatistics cacheStats; + private ExponentiallyWeightedMovingAverage getTimeMillisEWMA; + private static final double GET_TIME_EWMA_ALPHA = 0.3; // This is the value used elsewhere in the code // private RBMIntKeyLookupStore keystore; // private CacheTierPolicy[] policies; // private IndicesRequestCacheDiskTierPolicy policy; @@ -67,6 +72,9 @@ public EhcacheDiskCachingTier(long maxWeightInBytes, long maxKeystoreWeightInByt ).build(true); this.cache = cacheManager.getCache(cacheAlias, keyType, valueType); this.cacheStats = statsService.getCacheStatistics(cacheAlias); + this.getTimeMillisEWMA = new ExponentiallyWeightedMovingAverage(GET_TIME_EWMA_ALPHA, 10); + + // try and feed it an OpenSearch threadpool rather than its default ExecutionService? // this.keystore = new RBMIntKeyLookupStore((int) Math.pow(2, 28), maxKeystoreWeightInBytes); // this.policies = new CacheTierPolicy[]{ new IndicesRequestCacheTookTimePolicy(settings, clusterSettings) }; @@ -78,7 +86,11 @@ public V get(K key) { // do we need to do the future stuff? I don't think so? // if (keystore.contains(key.hashCode()) { - return cache.get(key); // do we want to somehow keep track of how long each cache.get takes? + long now = System.nanoTime(); + V value = cache.get(key); + double tookTimeMillis = ((double) (System.nanoTime() - now)) / 1000000; + getTimeMillisEWMA.addValue(tookTimeMillis); + return value; // } // return null; @@ -91,6 +103,7 @@ public void put(K key, V value) { // CheckDataResult policyResult = policy.checkData(value) // if (policyResult.isAccepted()) { cache.put(key, value); + // keystore.add(key.hashCode()); // else { do something with policyResult.deniedReason()? } // } } @@ -107,6 +120,7 @@ public void invalidate(K key) { // if (keystore.contains(key.hashCode()) { cache.remove(key); + // keystore.remove(key.hashCode()); // } } @@ -135,7 +149,7 @@ public Iterable keys() { @Override public int count() { // this might be an expensive disk-seek call. Might be better to keep track ourselves? - return (int) cacheStats.getTierStatistics().get("Disk").getMappings(); + return (int) getTierStats().getMappings(); } @Override @@ -148,9 +162,40 @@ public void onRemoval(RemovalNotification notification) { removalListener.onRemoval(notification); } + public double getTimeMillisEWMA() { + return getTimeMillisEWMA.getAverage(); + } + + private TierStatistics getTierStats() { + return cacheStats.getTierStatistics().get("Disk"); + } + + public long getHits() { + return getTierStats().getHits(); + } + + public long getMisses() { + return getTierStats().getMisses(); + } + + public long getWeightBytes() { + return getTierStats().getOccupiedByteSize(); + } + + public long getEvictions() { + return getTierStats().getEvictions(); + } + + public double getHitRatio() { + TierStatistics ts = getTierStats(); + long hits = ts.getHits(); + return hits / (hits + ts.getMisses()); + } + // See https://stackoverflow.com/questions/45827753/listenerobject-not-found-in-imports-for-ehcache-3 for API reference - // This class is used to get the old value from mutating calls to the cache and use those to create a RemovalNotification - private class EhcacheEventListener implements CacheEventListener { + // it's not actually documented by ehcache :( + // This class is used to get the old value from mutating calls to the cache, and it uses those to create a RemovalNotification + private class EhcacheEventListener implements CacheEventListener { // try making these specific, but i dont think itll work private RemovalListener removalListener; EhcacheEventListener(RemovalListener removalListener) { this.removalListener = removalListener; @@ -168,10 +213,9 @@ public void onEvent(CacheEvent event) { reason = RemovalReason.EVICTED; // why is there both RemovalReason.EVICTED and RemovalReason.CAPACITY? break; case EXPIRED: - reason = RemovalReason.INVALIDATED; // not sure about this one - break; case REMOVED: - reason = RemovalReason.INVALIDATED; // we use cache.remove() to invalidate keys, but this might overlap with RemovalReason.EXPLICIT? + reason = RemovalReason.INVALIDATED; + // this is probably fine for EXPIRED. We use cache.remove() to invalidate keys, but this might overlap with RemovalReason.EXPLICIT? break; case UPDATED: reason = RemovalReason.REPLACED; From dcb9fad0cc76162d61045d4ef87638f33d8a7698 Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Wed, 11 Oct 2023 14:17:42 -0700 Subject: [PATCH 05/22] thread pool stuff --- .../offheap-disk-store/ehcache-disk-store.meta | 2 +- .../indices/EhcacheDiskCachingTier.java | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.meta b/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.meta index 2d3e90d195f10..3165bdaf28d86 100644 --- a/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.meta +++ b/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.meta @@ -1,4 +1,4 @@ #Key and value types -#Tue Oct 10 19:13:45 EDT 2023 +#Wed Oct 11 23:48:40 GMT+03:00 2023 valueType=org.opensearch.core.common.bytes.BytesReference keyType=org.opensearch.indices.IndicesRequestCache$Key diff --git a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java index 3f9057c436be0..b28c159ffda17 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java @@ -11,6 +11,7 @@ import org.ehcache.config.builders.CacheConfigurationBuilder; import org.ehcache.config.builders.CacheEventListenerConfigurationBuilder; import org.ehcache.config.builders.CacheManagerBuilder; +import org.ehcache.config.builders.PooledExecutionServiceConfigurationBuilder; import org.ehcache.config.builders.ResourcePoolsBuilder; import org.ehcache.config.units.MemoryUnit; import org.ehcache.core.internal.statistics.DefaultStatisticsService; @@ -20,6 +21,7 @@ import org.ehcache.event.CacheEvent; import org.ehcache.event.CacheEventListener; import org.ehcache.event.EventType; +import org.ehcache.impl.config.executor.PooledExecutionServiceConfiguration; import org.opensearch.common.ExponentiallyWeightedMovingAverage; import org.opensearch.common.cache.RemovalListener; import org.ehcache.Cache; @@ -28,7 +30,6 @@ import org.opensearch.common.cache.RemovalReason; import java.util.Collections; -import java.util.Map; public class EhcacheDiskCachingTier implements CachingTier, RemovalListener { @@ -39,10 +40,11 @@ public class EhcacheDiskCachingTier implements CachingTier, RemovalL private final Class valueType; private final String DISK_CACHE_FP = "disk_cache_tier"; // this should probably be defined somewhere else since we need to change security.policy based on its value private RemovalListener removalListener; - private final StatisticsService statsService; private final CacheStatistics cacheStats; private ExponentiallyWeightedMovingAverage getTimeMillisEWMA; private static final double GET_TIME_EWMA_ALPHA = 0.3; // This is the value used elsewhere in the code + private static final int MIN_WRITE_THREADS = 0; + private static final int MAX_WRITE_THREADS = 4; // Max number of threads for the PooledExecutionService which handles writes // private RBMIntKeyLookupStore keystore; // private CacheTierPolicy[] policies; // private IndicesRequestCacheDiskTierPolicy policy; @@ -52,7 +54,7 @@ public EhcacheDiskCachingTier(long maxWeightInBytes, long maxKeystoreWeightInByt this.keyType = keyType; this.valueType = valueType; String cacheAlias = "diskTier"; - this.statsService = new DefaultStatisticsService(); + StatisticsService statsService = new DefaultStatisticsService(); // our EhcacheEventListener should receive events every time an entry is removed for some reason, but not when it's created CacheEventListenerConfigurationBuilder listenerConfig = CacheEventListenerConfigurationBuilder @@ -63,12 +65,18 @@ public EhcacheDiskCachingTier(long maxWeightInBytes, long maxKeystoreWeightInByt EventType.UPDATED) .ordered().asynchronous(); // ordered() has some performance penalty as compared to unordered(), we can also use synchronous() + // test PooledExecutionService so we know what we might need to replicate + PooledExecutionServiceConfiguration threadConfig = PooledExecutionServiceConfigurationBuilder.newPooledExecutionServiceConfigurationBuilder() + .defaultPool("default", MIN_WRITE_THREADS, MAX_WRITE_THREADS) + .build(); + this.cacheManager = CacheManagerBuilder.newCacheManagerBuilder() .using(statsService) + .using(threadConfig) .with(CacheManagerBuilder.persistence(DISK_CACHE_FP)) .withCache(cacheAlias, CacheConfigurationBuilder.newCacheConfigurationBuilder( keyType, valueType, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(maxWeightInBytes, MemoryUnit.B, true)) - .withService(listenerConfig) + .withService(listenerConfig) // stackoverflow shows .add(), but IDE says this is deprecated. idk ).build(true); this.cache = cacheManager.getCache(cacheAlias, keyType, valueType); this.cacheStats = statsService.getCacheStatistics(cacheAlias); From b912527b129f0bc48571c87e2686783fb04299e0 Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Wed, 11 Oct 2023 15:13:17 -0700 Subject: [PATCH 06/22] cleanup --- ...CompactJavaSerializer-ObjectStreamClassIndex.bin | Bin 0 -> 4 bytes .../opensearch/indices/EhcacheDiskCachingTier.java | 6 ++---- 2 files changed, 2 insertions(+), 4 deletions(-) create mode 100644 server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/value-serializer/holder-0-CompactJavaSerializer-ObjectStreamClassIndex.bin diff --git a/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/value-serializer/holder-0-CompactJavaSerializer-ObjectStreamClassIndex.bin b/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/value-serializer/holder-0-CompactJavaSerializer-ObjectStreamClassIndex.bin new file mode 100644 index 0000000000000000000000000000000000000000..711006c3d3b5c6d50049e3f48311f3dbe372803d GIT binary patch literal 4 LcmZ4UmVp%j1%Lsc literal 0 HcmV?d00001 diff --git a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java index b28c159ffda17..2e69fd372c7c6 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java @@ -42,13 +42,12 @@ public class EhcacheDiskCachingTier implements CachingTier, RemovalL private RemovalListener removalListener; private final CacheStatistics cacheStats; private ExponentiallyWeightedMovingAverage getTimeMillisEWMA; - private static final double GET_TIME_EWMA_ALPHA = 0.3; // This is the value used elsewhere in the code + private static final double GET_TIME_EWMA_ALPHA = 0.3; // This is the value used elsewhere in OpenSearch private static final int MIN_WRITE_THREADS = 0; private static final int MAX_WRITE_THREADS = 4; // Max number of threads for the PooledExecutionService which handles writes // private RBMIntKeyLookupStore keystore; // private CacheTierPolicy[] policies; // private IndicesRequestCacheDiskTierPolicy policy; - //private RemovalListener removalListener = notification -> {}; // based on on-heap public EhcacheDiskCachingTier(long maxWeightInBytes, long maxKeystoreWeightInBytes, Class keyType, Class valueType) { this.keyType = keyType; @@ -56,7 +55,7 @@ public EhcacheDiskCachingTier(long maxWeightInBytes, long maxKeystoreWeightInByt String cacheAlias = "diskTier"; StatisticsService statsService = new DefaultStatisticsService(); - // our EhcacheEventListener should receive events every time an entry is removed for some reason, but not when it's created + // our EhcacheEventListener should receive events every time an entry is removed, but not when it's created CacheEventListenerConfigurationBuilder listenerConfig = CacheEventListenerConfigurationBuilder .newEventListenerConfiguration(new EhcacheEventListener(this), EventType.EVICTED, @@ -65,7 +64,6 @@ public EhcacheDiskCachingTier(long maxWeightInBytes, long maxKeystoreWeightInByt EventType.UPDATED) .ordered().asynchronous(); // ordered() has some performance penalty as compared to unordered(), we can also use synchronous() - // test PooledExecutionService so we know what we might need to replicate PooledExecutionServiceConfiguration threadConfig = PooledExecutionServiceConfigurationBuilder.newPooledExecutionServiceConfigurationBuilder() .defaultPool("default", MIN_WRITE_THREADS, MAX_WRITE_THREADS) .build(); From 44d5e6105ff92bf97481c9612d6429d7af3cdab9 Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Wed, 11 Oct 2023 15:23:53 -0700 Subject: [PATCH 07/22] Removed partial cache files from disk --- server/disk_cache_tier/.lock | 0 .../offheap-disk-store/ehcache-disk-store.data | Bin 16384 -> 0 bytes .../offheap-disk-store/ehcache-disk-store.meta | 4 ---- ...actJavaSerializer-ObjectStreamClassIndex.bin | Bin 4 -> 0 bytes 4 files changed, 4 deletions(-) delete mode 100644 server/disk_cache_tier/.lock delete mode 100644 server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.data delete mode 100644 server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.meta delete mode 100644 server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/value-serializer/holder-0-CompactJavaSerializer-ObjectStreamClassIndex.bin diff --git a/server/disk_cache_tier/.lock b/server/disk_cache_tier/.lock deleted file mode 100644 index e69de29bb2d1d..0000000000000 diff --git a/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.data b/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.data deleted file mode 100644 index 294f4016d05bdd696670c4840f1f36a71f9239de..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 16384 zcmeIuF#!Mo0K%a4Pi+hzh(KY$fB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM q7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjMyaxtA00031 diff --git a/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.meta b/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.meta deleted file mode 100644 index 3165bdaf28d86..0000000000000 --- a/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.meta +++ /dev/null @@ -1,4 +0,0 @@ -#Key and value types -#Wed Oct 11 23:48:40 GMT+03:00 2023 -valueType=org.opensearch.core.common.bytes.BytesReference -keyType=org.opensearch.indices.IndicesRequestCache$Key diff --git a/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/value-serializer/holder-0-CompactJavaSerializer-ObjectStreamClassIndex.bin b/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/value-serializer/holder-0-CompactJavaSerializer-ObjectStreamClassIndex.bin deleted file mode 100644 index 711006c3d3b5c6d50049e3f48311f3dbe372803d..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 4 LcmZ4UmVp%j1%Lsc From 43e2d1deb4596cf798c4dfed52a2ea5147702f95 Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Wed, 11 Oct 2023 16:14:50 -0700 Subject: [PATCH 08/22] fixed double counting of on heap hit count --- .../opensearch/indices/TieredCacheSpilloverStrategyHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java b/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java index 1047f8a6dc2cc..b1b68bbf93f79 100644 --- a/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java +++ b/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java @@ -54,7 +54,7 @@ public V computeIfAbsent(K key, TieredCacheLoader loader) throws Exception tieredCacheEventListener.onCached(key, value, TierType.ON_HEAP); return value; } else { - tieredCacheEventListener.onHit(key, cacheValue.value, cacheValue.source); + //tieredCacheEventListener.onHit(key, cacheValue.value, cacheValue.source); // this double counts, see line 122 } return cacheValue.value; } From aa2d28d8fef2a02bcfae81cf3aad80d29e818381 Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Wed, 11 Oct 2023 16:38:03 -0700 Subject: [PATCH 09/22] first attempt at fixing cache closing issue in tests --- server/disk_cache_tier/.lock | 0 ...actJavaSerializer-ObjectStreamClassIndex.bin | Bin 0 -> 1367 bytes .../offheap-disk-store/ehcache-disk-store.data | Bin 0 -> 16384 bytes .../offheap-disk-store/ehcache-disk-store.index | Bin 0 -> 473 bytes .../offheap-disk-store/ehcache-disk-store.meta | 4 ++++ ...actJavaSerializer-ObjectStreamClassIndex.bin | Bin 0 -> 1367 bytes .../indices/EhcacheDiskCachingTier.java | 7 +++++++ .../opensearch/indices/IndicesRequestCache.java | 4 ++++ .../opensearch/indices/TieredCacheHandler.java | 2 ++ .../TieredCacheSpilloverStrategyHandler.java | 11 ++++++++--- .../org/opensearch/bootstrap/security.policy | 2 ++ .../indices/IndicesRequestCacheTests.java | 6 ++++++ 12 files changed, 33 insertions(+), 3 deletions(-) create mode 100644 server/disk_cache_tier/.lock create mode 100644 server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/key-serializer/holder-0-CompactJavaSerializer-ObjectStreamClassIndex.bin create mode 100644 server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.data create mode 100644 server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.index create mode 100644 server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.meta create mode 100644 server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/value-serializer/holder-0-CompactJavaSerializer-ObjectStreamClassIndex.bin diff --git a/server/disk_cache_tier/.lock b/server/disk_cache_tier/.lock new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/key-serializer/holder-0-CompactJavaSerializer-ObjectStreamClassIndex.bin b/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/key-serializer/holder-0-CompactJavaSerializer-ObjectStreamClassIndex.bin new file mode 100644 index 0000000000000000000000000000000000000000..ef98475e283358e20439f5c5846b5c71b003ab05 GIT binary patch literal 1367 zcmeHG&ubGw6n;&tS}RyQi(Ud6NO!?%5M!-15*pcB%vKP4nrvRWlkU!}Gqakic=4b| z!CnNtc=1=os~$v;g8mKd!Gj0!fADRxX;WHD@N5oCX4&t@`@VUrzhT%wp)A9QdFC>^ z#avXJ1^f=G@ws{nJ>wdMEG!31>#AsATPr9@wP>O5G0#Vfhf!cfs5I9Gg@;y!2Y8Wb zY`BIQtYIWIH&QL<=Hn>9kuzV0K8>uz8OT&&m?w-5?Db{PKxH8B5Vf6B+dATQAG6&d zoaua8YDYjnGBdHwj7~>jhD%}1D<(8YL4%N;F8bx#gSXGlmew;cM0Q=LUHZZ-gC6FX*yQj9Z%~UPtb_KdQ9ipg?#44rRT3wHBSMwWm zaq;-K)qUr_X8<}4j8P!x|1FRn=dq&m`1+IlB$ZKt1A%0X3f)Di+6qo2omfdH{q6$k zEorNUlpl3#96C0yy($#GwtjUA-J_RYwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM q7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjMyaxtA00031 literal 0 HcmV?d00001 diff --git a/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.index b/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.index new file mode 100644 index 0000000000000000000000000000000000000000..f7bb9ed1e3f24bf13bc8596347fb76e10b61d7a7 GIT binary patch literal 473 zcmZ4UmVvd3fr0S?5O*s|N=yoO4EAJTU=Z;1@d43LzyhKL9DQ9}K@5Z(NF0|O2Oc>d sJaPhf^ z#avXJ1^f=G@ws{nJ>wdMEG!31>#AsATPr9@wP>O5G0#Vfhf!cfs5I9Gg@;y!2Y8Wb zY`BIQtYIWIH&QL<=Hn>9kuzV0K8>uz8OT&&m?w-5?Db{PKxH8B5Vf6B+dATQAG6&d zoaua8YDYjnGBdHwj7~>jhD%}1D<(8YL4%N;F8bx#gSXGlmew;cM0Q=LUHZZ-gC6FX*yQj9Z%~UPtb_KdQ9ipg?#44rRT3wHBSMwWm zaq;-K)qUr_X8<}4j8P!x|1FRn=dq&m`1+IlB$ZKt1A%0X3f)Di+6qo2omfdH{q6$k zEorNUlpl3#96C0yy($#GwtjUA-J_R { long count(); CachingTier getOnHeapCachingTier(); + + void closeDiskTier(); } diff --git a/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java b/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java index b1b68bbf93f79..a2df5545eff6b 100644 --- a/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java +++ b/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java @@ -25,7 +25,7 @@ public class TieredCacheSpilloverStrategyHandler implements TieredCacheHandler, RemovalListener { private final OnHeapCachingTier onHeapCachingTier; - private final CachingTier diskCachingTier; + private final EhcacheDiskCachingTier diskCachingTier; // changed for testing private final TieredCacheEventListener tieredCacheEventListener; /** @@ -35,7 +35,8 @@ public class TieredCacheSpilloverStrategyHandler implements TieredCacheHan private TieredCacheSpilloverStrategyHandler( OnHeapCachingTier onHeapCachingTier, - CachingTier diskCachingTier, + EhcacheDiskCachingTier diskCachingTier, + // changed to EhcacheDiskCachingTier from CachingTier, to enable close() method, which is needed for tests. Implement close() in CachingTier or DiskCachingTier or something? TieredCacheEventListener tieredCacheEventListener ) { this.onHeapCachingTier = Objects.requireNonNull(onHeapCachingTier); @@ -127,6 +128,10 @@ private Function> getValueFromTierCache() { return null; }; } + @Override + public void closeDiskTier() { + diskCachingTier.close(); + } public static class CacheValue { V value; @@ -163,7 +168,7 @@ public Builder setTieredCacheEventListener(TieredCacheEventListener public TieredCacheSpilloverStrategyHandler build() { return new TieredCacheSpilloverStrategyHandler( this.onHeapCachingTier, - this.diskCachingTier, + (EhcacheDiskCachingTier) this.diskCachingTier, // not sure why it was yelling about this, it already is an EhcacheDiskCachingTier this.tieredCacheEventListener ); } diff --git a/server/src/main/resources/org/opensearch/bootstrap/security.policy b/server/src/main/resources/org/opensearch/bootstrap/security.policy index 20e5671d582a6..1fbfbb323e3af 100644 --- a/server/src/main/resources/org/opensearch/bootstrap/security.policy +++ b/server/src/main/resources/org/opensearch/bootstrap/security.policy @@ -194,6 +194,8 @@ grant { permission java.lang.RuntimePermission "getenv.*"; permission java.io.FilePermission "disk_cache_tier", "read"; // change this to wherever we will put disk tier folder permission java.io.FilePermission "disk_cache_tier", "write"; + permission java.io.FilePermission "disk_cache_tier", "delete"; permission java.io.FilePermission "disk_cache_tier/-", "read"; permission java.io.FilePermission "disk_cache_tier/-", "write"; + permission java.io.FilePermission "disk_cache_tier/-", "delete"; }; diff --git a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java index 8494259c8fd8a..5318a4f16d8f3 100644 --- a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java +++ b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java @@ -119,6 +119,7 @@ public void testBasicOperationsCache() throws Exception { IOUtils.close(reader, writer, dir, cache); assertEquals(0, cache.numRegisteredCloseListeners()); + cache.closeDiskTier(); } public void testCacheDifferentReaders() throws Exception { @@ -213,6 +214,7 @@ public void testCacheDifferentReaders() throws Exception { IOUtils.close(secondReader, writer, dir, cache); assertEquals(0, cache.numRegisteredCloseListeners()); + cache.closeDiskTier(); } public void testEviction() throws Exception { @@ -242,6 +244,7 @@ public void testEviction() throws Exception { assertEquals("bar", value2.streamInput().readString()); size = requestCacheStats.stats().getMemorySize(); IOUtils.close(reader, secondReader, writer, dir, cache); + cache.closeDiskTier(); } IndicesRequestCache cache = new IndicesRequestCache( Settings.builder().put(IndicesRequestCache.INDICES_CACHE_QUERY_SIZE.getKey(), size.getBytes() + 1 + "b").build() @@ -278,6 +281,7 @@ public void testEviction() throws Exception { assertEquals(2, cache.count()); assertEquals(1, requestCacheStats.stats().getEvictions()); IOUtils.close(reader, secondReader, thirdReader, writer, dir, cache); + cache.closeDiskTier(); } public void testClearAllEntityIdentity() throws Exception { @@ -325,6 +329,7 @@ public void testClearAllEntityIdentity() throws Exception { assertEquals("baz", value3.streamInput().readString()); IOUtils.close(reader, secondReader, thirdReader, writer, dir, cache); + cache.closeDiskTier(); } @@ -430,6 +435,7 @@ public void testInvalidate() throws Exception { IOUtils.close(reader, writer, dir, cache); assertEquals(0, cache.numRegisteredCloseListeners()); + cache.closeDiskTier(); } public void testEqualsKey() throws IOException { From 354c311dbde2fa96420da2c17920bc503dbcd815 Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Thu, 12 Oct 2023 15:11:50 -0700 Subject: [PATCH 10/22] misc stuff for testing --- server/disk_cache_tier/.lock | 0 ...tJavaSerializer-ObjectStreamClassIndex.bin | Bin 1367 -> 3081 bytes .../ehcache-disk-store.index | Bin 473 -> 473 bytes .../ehcache-disk-store.meta | 2 +- .../cache/request/ShardRequestCache.java | 31 +++++- .../indices/EhcacheDiskCachingTier.java | 93 ++++++++++++------ .../indices/IndicesRequestCache.java | 6 +- .../TieredCacheSpilloverStrategyHandler.java | 4 + .../indices/IndicesRequestCacheTests.java | 69 +++++++++++++ 9 files changed, 167 insertions(+), 38 deletions(-) delete mode 100644 server/disk_cache_tier/.lock diff --git a/server/disk_cache_tier/.lock b/server/disk_cache_tier/.lock deleted file mode 100644 index e69de29bb2d1d..0000000000000 diff --git a/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/key-serializer/holder-0-CompactJavaSerializer-ObjectStreamClassIndex.bin b/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/key-serializer/holder-0-CompactJavaSerializer-ObjectStreamClassIndex.bin index ef98475e283358e20439f5c5846b5c71b003ab05..a8f701c5f90c8437b80ab6beaadff9b5cc26b90b 100644 GIT binary patch literal 3081 zcmeHJU1%It6u#L_()?H(;~!|-h^)4ebOwzHiJP`I+k~!e65NeN%+t-x+1*KI=1%Xu z)9h9eMQjBT1mlCEMMS9Dh%Y|G2T?Evl-PxZeh9E|Ka36Me z?ws?TbH4L;SAHdJk_@Dn7f?$+LQm5iaEH3i8Q_Oz<`=+{Imv-~DVGY7_H8H$PLhll z3@BREDuUs7folZ7g(D>R78sL`3*%IPos(2T76K+5$#^+28wM_Pj(pzoY3FhnBe67T za};8Rv=^Cc1DB*fgV1Ib+7y9fHXxfC@PrmkmI8twbY^gaGol&-l@9ZbSx$Z7fRBNo zI~DB<>(5_&^P%LG7-`9nxJLs?5}5^z-VCMVnilh|kaN^AQ?Kb)Lqpo;@iXtl8udnyMb*}$#oW#?lTR_1>{3$9H)8vNgQ?BUbW%{39C zxHcneo4Z-HmdI5MRCwBhRP7CU6|OmT5!{x}*Wkk81D98JjeQ*>L}#D}2|4oj2#GNJ zhBL1`@oN8JOvWVXav8NJu@)IGadJ@Q#85eD^v-40qA;?6{9$fo1|Rc}zc(=O)xs}2 zp{=6qTauWYXH;vOP1=bZDpZpD;C3x0rkS6o4p*x1;saN6Pkq*{Na5trwjq@6>i&0@ zKREv6(-qyJEp(5~3yjy2kA)CobD?gLGhcNoMxP3gW`D%!v8RNpbM#|)8(1?-KAm@= z5p3|vN1uG?vG=14lcNt~pPeGlV414#9@TgMpKZYbCpT&8UFsK%G|q+s@ZKNZe)sq1 z&W%)3beg)MN>NW;ZaVblfuCJ@ZD05D_b=7R=fqIm#RBjJP;M0s9GQ*<0)h2!7EXmA zWJ=BA#G_E2T$4YqE%#oEkvPs(Y!OG6B^l9Uq>h3^L~EOyp=hiLt(nY_8*%*OY}SW} zFNvYCC@zd5m~Lo+4+;)S3JgkFEuM(v5ur_oE6M%WJ7F;nIK@1V`R06Cf-uL`cNX%1 z!`A}UlB(nevEn*7>8lrXin%WCCY-*W}d{TR6E(D>}@W zW-{7us_Z9976c09;K>rXdiJ)VN)zsj#2Xi2P1gS6r^S~?|G?d$n*HWTHa%05aGw=6 z-1FPTMhRUDHHmMo^&Pn*xp!@Y#4NV-l5*|aOG5`nmsT&wNh=a)(NKUSw^k;a!ZK@; zNbqhOBz4>=5P}v^uD3xB*SF&rl5?aB=Yq9}+t5OZ{c}ws(TfQe)K)^sTSOQH909?u zd5!z1r(vcl${RBjuFMlsU}*~$??AkUA>!VD98A}n))3Gpfw8MAE?Y7 zaIdY&y;`wv(iS^lyq!l4VrX{o=2-2Vs6k?r_l=ERyL(hMaD7JY;^a;}4>s4LonO9j z@9=jo#7M_DY0oq6QJKaYkK;pS`e(=w9?hv?Gr< h?FOkX4`h+qO()C_n`Cx-`sAd#uQOi*m4YBB{R#TT{U-nb delta 11 ScmeB_xX!g9gq69VpaK9GQUm7z diff --git a/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.index b/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.index index f7bb9ed1e3f24bf13bc8596347fb76e10b61d7a7..05ab5af78b038e9fc54d3d0ee751c5b0e700edd0 100644 GIT binary patch delta 26 gcmcb~e3Myl&07Z6Dh39|3qahh`u?@-M!^$|0C#N&<^TWy delta 26 gcmcb~e3Myl&07Z6Dh39|3qahhC@C>%qu>cf0B*_$t^fc4 diff --git a/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.meta b/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.meta index bc22293f99d4d..acd184629f922 100644 --- a/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.meta +++ b/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.meta @@ -1,4 +1,4 @@ #Key and value types -#Wed Oct 11 21:36:14 WGST 2023 +#Thu Oct 12 16:18:26 EDT 2023 valueType=org.opensearch.core.common.bytes.BytesReference keyType=org.opensearch.indices.IndicesRequestCache$Key diff --git a/server/src/main/java/org/opensearch/index/cache/request/ShardRequestCache.java b/server/src/main/java/org/opensearch/index/cache/request/ShardRequestCache.java index 1beef5217355f..f09d2504680c7 100644 --- a/server/src/main/java/org/opensearch/index/cache/request/ShardRequestCache.java +++ b/server/src/main/java/org/opensearch/index/cache/request/ShardRequestCache.java @@ -56,11 +56,34 @@ public ShardRequestCache() { public RequestCacheStats stats() { // TODO: Change RequestCacheStats to support disk tier stats. + return stats(TierType.ON_HEAP); + } + + public RequestCacheStats stats(TierType tierType) { + return new RequestCacheStats( + statsHolder.get(tierType).totalMetric.count(), + statsHolder.get(tierType).evictionsMetric.count(), + statsHolder.get(tierType).hitCount.count(), + statsHolder.get(tierType).missCount.count() + ); + } + + public RequestCacheStats overallStats() { + long totalSize = 0; + long totalEvictions = 0; + long totalHits = 0; + long totalMisses = 0; + for (TierType tierType : TierType.values()) { + totalSize += statsHolder.get(tierType).totalMetric.count(); + totalEvictions += statsHolder.get(tierType).evictionsMetric.count(); + totalHits += statsHolder.get(tierType).hitCount.count(); + totalMisses += statsHolder.get(tierType).missCount.count(); + } return new RequestCacheStats( - statsHolder.get(TierType.ON_HEAP).totalMetric.count(), - statsHolder.get(TierType.ON_HEAP).evictionsMetric.count(), - statsHolder.get(TierType.ON_HEAP).hitCount.count(), - statsHolder.get(TierType.ON_HEAP).missCount.count() + totalSize, + totalEvictions, + totalHits, + totalMisses ); } diff --git a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java index 2e33eb3b05b64..8fc62fe02bd9a 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java @@ -8,6 +8,7 @@ package org.opensearch.indices; +import org.ehcache.PersistentCacheManager; import org.ehcache.config.builders.CacheConfigurationBuilder; import org.ehcache.config.builders.CacheEventListenerConfigurationBuilder; import org.ehcache.config.builders.CacheManagerBuilder; @@ -16,7 +17,6 @@ import org.ehcache.config.units.MemoryUnit; import org.ehcache.core.internal.statistics.DefaultStatisticsService; import org.ehcache.core.spi.service.StatisticsService; -import org.ehcache.core.statistics.CacheStatistics; import org.ehcache.core.statistics.TierStatistics; import org.ehcache.event.CacheEvent; import org.ehcache.event.CacheEventListener; @@ -25,7 +25,6 @@ import org.opensearch.common.ExponentiallyWeightedMovingAverage; import org.opensearch.common.cache.RemovalListener; import org.ehcache.Cache; -import org.ehcache.CacheManager; import org.opensearch.common.cache.RemovalNotification; import org.opensearch.common.cache.RemovalReason; @@ -33,55 +32,57 @@ public class EhcacheDiskCachingTier implements CachingTier, RemovalListener { - private CacheManager cacheManager; - private final Cache cache; + private final PersistentCacheManager cacheManager; + public final Cache cache; // make private after debug private final Class keyType; // I think these are needed to pass to newCacheConfigurationBuilder private final Class valueType; private final String DISK_CACHE_FP = "disk_cache_tier"; // this should probably be defined somewhere else since we need to change security.policy based on its value private RemovalListener removalListener; - private final CacheStatistics cacheStats; + private StatisticsService statsService; // non functional private ExponentiallyWeightedMovingAverage getTimeMillisEWMA; private static final double GET_TIME_EWMA_ALPHA = 0.3; // This is the value used elsewhere in OpenSearch private static final int MIN_WRITE_THREADS = 0; private static final int MAX_WRITE_THREADS = 4; // Max number of threads for the PooledExecutionService which handles writes + private static final String cacheAlias = "diskTier"; + private final boolean isPersistent; + private int count; // number of entries in cache // private RBMIntKeyLookupStore keystore; // private CacheTierPolicy[] policies; // private IndicesRequestCacheDiskTierPolicy policy; - public EhcacheDiskCachingTier(long maxWeightInBytes, long maxKeystoreWeightInBytes, Class keyType, Class valueType) { + public EhcacheDiskCachingTier(boolean isPersistent, long maxWeightInBytes, long maxKeystoreWeightInBytes, Class keyType, Class valueType) { this.keyType = keyType; this.valueType = valueType; - String cacheAlias = "diskTier"; - StatisticsService statsService = new DefaultStatisticsService(); + this.isPersistent = isPersistent; + this.count = 0; + statsService = new DefaultStatisticsService(); - // our EhcacheEventListener should receive events every time an entry is removed, but not when it's created + // our EhcacheEventListener should receive events every time an entry is changed CacheEventListenerConfigurationBuilder listenerConfig = CacheEventListenerConfigurationBuilder - .newEventListenerConfiguration(new EhcacheEventListener(this), + .newEventListenerConfiguration(new EhcacheEventListener(this, this), EventType.EVICTED, EventType.EXPIRED, EventType.REMOVED, - EventType.UPDATED) - .ordered().asynchronous(); // ordered() has some performance penalty as compared to unordered(), we can also use synchronous() + EventType.UPDATED, + EventType.CREATED); + //.ordered().asynchronous(); // ordered() has some performance penalty as compared to unordered(), we can also use synchronous() PooledExecutionServiceConfiguration threadConfig = PooledExecutionServiceConfigurationBuilder.newPooledExecutionServiceConfigurationBuilder() .defaultPool("default", MIN_WRITE_THREADS, MAX_WRITE_THREADS) .build(); this.cacheManager = CacheManagerBuilder.newCacheManagerBuilder() - .using(statsService) + .using(statsService) // https://stackoverflow.com/questions/40453859/how-to-get-ehcache-3-1-statistics .using(threadConfig) .with(CacheManagerBuilder.persistence(DISK_CACHE_FP)) .withCache(cacheAlias, CacheConfigurationBuilder.newCacheConfigurationBuilder( - keyType, valueType, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(maxWeightInBytes, MemoryUnit.B, true)) + keyType, valueType, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(maxWeightInBytes, MemoryUnit.B, isPersistent)) .withService(listenerConfig) // stackoverflow shows .add(), but IDE says this is deprecated. idk ).build(true); this.cache = cacheManager.getCache(cacheAlias, keyType, valueType); - this.cacheStats = statsService.getCacheStatistics(cacheAlias); this.getTimeMillisEWMA = new ExponentiallyWeightedMovingAverage(GET_TIME_EWMA_ALPHA, 10); - // try and feed it an OpenSearch threadpool rather than its default ExecutionService? - // this.keystore = new RBMIntKeyLookupStore((int) Math.pow(2, 28), maxKeystoreWeightInBytes); // this.policies = new CacheTierPolicy[]{ new IndicesRequestCacheTookTimePolicy(settings, clusterSettings) }; // this.policy = new IndicesRequestCacheDiskTierPolicy(this.policies, true); @@ -109,6 +110,7 @@ public void put(K key, V value) { // CheckDataResult policyResult = policy.checkData(value) // if (policyResult.isAccepted()) { cache.put(key, value); + //count++; // keystore.add(key.hashCode()); // else { do something with policyResult.deniedReason()? } // } @@ -126,6 +128,7 @@ public void invalidate(K key) { // if (keystore.contains(key.hashCode()) { cache.remove(key); + //count--; // keystore.remove(key.hashCode()); // } } @@ -154,8 +157,15 @@ public Iterable keys() { @Override public int count() { - // this might be an expensive disk-seek call. Might be better to keep track ourselves? - return (int) getTierStats().getMappings(); + return count; + //return (int) getTierStats().getMappings(); + } + + protected void countInc() { + count++; + } + protected void countDec() { + count--; } @Override @@ -172,8 +182,11 @@ public double getTimeMillisEWMA() { return getTimeMillisEWMA.getAverage(); } - private TierStatistics getTierStats() { - return cacheStats.getTierStatistics().get("Disk"); + // these aren't really needed, ShardRequestCache handles it + // Also, it seems that ehcache doesn't have functioning statistics anyway! + + /*public TierStatistics getTierStats() { + return statsService.getCacheStatistics(cacheAlias).getTierStatistics().get("Disk"); } public long getHits() { @@ -196,11 +209,16 @@ public double getHitRatio() { TierStatistics ts = getTierStats(); long hits = ts.getHits(); return hits / (hits + ts.getMisses()); + }*/ + + public boolean isPersistent() { + return isPersistent; } public void close() { // Call this method after each test, otherwise the directory will stay locked and you won't be able to - // initialize another IndicesRequestCache + // initialize another IndicesRequestCache (for example in the next test that runs) + cacheManager.removeCache(cacheAlias); cacheManager.close(); } @@ -208,20 +226,36 @@ public void close() { // See https://stackoverflow.com/questions/45827753/listenerobject-not-found-in-imports-for-ehcache-3 for API reference // it's not actually documented by ehcache :( // This class is used to get the old value from mutating calls to the cache, and it uses those to create a RemovalNotification - private class EhcacheEventListener implements CacheEventListener { // try making these specific, but i dont think itll work + // It also handles incrementing and decrementing the count for the disk tier, since ehcache's statistics functionality + // does not seem to work + private class EhcacheEventListener implements CacheEventListener { // try making these specific, but i dont think itll work private RemovalListener removalListener; - EhcacheEventListener(RemovalListener removalListener) { + private EhcacheDiskCachingTier tier; + EhcacheEventListener(RemovalListener removalListener, EhcacheDiskCachingTier tier) { this.removalListener = removalListener; + this.tier = tier; // needed to handle count changes } @Override - public void onEvent(CacheEvent event) { - // send a RemovalNotification - K key = (K) event.getKey(); // I think these casts should be ok? - V oldValue = (V) event.getOldValue(); - V newValue = (V) event.getNewValue(); + public void onEvent(CacheEvent event) { + K key = event.getKey(); + V oldValue = event.getOldValue(); + V newValue = event.getNewValue(); EventType eventType = event.getType(); + + System.out.println("I am eventing!!"); + + // handle changing count for the disk tier + if (oldValue == null && newValue != null) { + tier.countInc(); + } else if (oldValue != null && newValue == null) { + tier.countDec(); + } + + // handle creating a RemovalReason, unless eventType is CREATED RemovalReason reason; switch (eventType) { + case CREATED: + return; case EVICTED: reason = RemovalReason.EVICTED; // why is there both RemovalReason.EVICTED and RemovalReason.CAPACITY? break; @@ -236,7 +270,6 @@ public void onEvent(CacheEvent event) { default: reason = null; } - // we don't subscribe to CREATED type, which is the only other option removalListener.onRemoval(new RemovalNotification(key, oldValue, reason)); } } diff --git a/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java b/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java index d92e139270dec..9a475be609a1a 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java +++ b/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java @@ -107,8 +107,8 @@ public final class IndicesRequestCache implements TieredCacheEventListener cache; - private final TieredCacheHandler tieredCacheHandler; - + //private final TieredCacheHandler tieredCacheHandler; // made public TieredCacheSpilloverStrategyHandler for testing + public final TieredCacheSpilloverStrategyHandler tieredCacheHandler; IndicesRequestCache(Settings settings) { this.size = INDICES_CACHE_QUERY_SIZE.get(settings); this.expire = INDICES_CACHE_QUERY_EXPIRE.exists(settings) ? INDICES_CACHE_QUERY_EXPIRE.get(settings) : null; @@ -130,7 +130,7 @@ public final class IndicesRequestCache implements TieredCacheEventListener diskCachingTier; - diskCachingTier = new EhcacheDiskCachingTier<>(diskTierWeight, 0, Key.class, BytesReference.class); + diskCachingTier = new EhcacheDiskCachingTier<>(true, diskTierWeight, 0, Key.class, BytesReference.class); tieredCacheHandler = new TieredCacheSpilloverStrategyHandler.Builder().setOnHeapCachingTier( openSearchOnHeapCache ).setOnDiskCachingTier(diskCachingTier).setTieredCacheEventListener(this).build(); diff --git a/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java b/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java index a2df5545eff6b..5657e3519f2ca 100644 --- a/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java +++ b/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java @@ -109,6 +109,10 @@ public CachingTier getOnHeapCachingTier() { return this.onHeapCachingTier; } + public EhcacheDiskCachingTier getDiskCachingTier() { // change to CachingTier after debug + return this.diskCachingTier; + } + private void setRemovalListeners() { for (CachingTier cachingTier : cachingTierList) { cachingTier.setRemovalListener(this); diff --git a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java index 5318a4f16d8f3..e8ad68c67d9c8 100644 --- a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java +++ b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java @@ -45,6 +45,7 @@ import org.apache.lucene.search.TopDocs; import org.apache.lucene.store.Directory; import org.apache.lucene.util.BytesRef; +import org.ehcache.core.statistics.TierStatistics; import org.opensearch.common.CheckedSupplier; import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.lucene.index.OpenSearchDirectoryReader; @@ -122,6 +123,74 @@ public void testBasicOperationsCache() throws Exception { cache.closeDiskTier(); } + public void testAddDirectToEhcache() throws Exception { + ShardRequestCache requestCacheStats = new ShardRequestCache(); + Settings.Builder settingsBuilder = Settings.builder(); + long heapSizeBytes = 1000; + settingsBuilder.put("indices.requests.cache.size", new ByteSizeValue(heapSizeBytes)); + IndicesRequestCache cache = new IndicesRequestCache(settingsBuilder.build()); + + // set up a key + Directory dir = newDirectory(); + IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig()); + writer.addDocument(newDoc(0, "foo")); + DirectoryReader reader = OpenSearchDirectoryReader.wrap(DirectoryReader.open(writer), new ShardId("foo", "bar", 1)); + AtomicBoolean indexShard = new AtomicBoolean(true); + TestEntity entity = new TestEntity(requestCacheStats, indexShard); + Loader loader = new Loader(reader, 0); + IndicesRequestCache.Key[] keys = new IndicesRequestCache.Key[9]; + TermQueryBuilder termQuery = new TermQueryBuilder("id", "0"); + BytesReference termBytes = XContentHelper.toXContent(termQuery, MediaTypeRegistry.JSON, false); + IndicesRequestCache.Key key = new IndicesRequestCache.Key(entity, reader.getReaderCacheHelper().getKey(), termBytes); + + TestBytesReference value = new TestBytesReference(124); + cache.tieredCacheHandler.getDiskCachingTier().cache.put(key, value); + + IOUtils.close(reader, writer, dir, cache); + cache.closeDiskTier(); + } + + public void testSpillover() throws Exception { + // fill the on-heap cache until we spill over + ShardRequestCache requestCacheStats = new ShardRequestCache(); + Settings.Builder settingsBuilder = Settings.builder(); + long heapSizeBytes = 1000; // each of these queries is 115 bytes, so we can fit 8 in the heap cache + settingsBuilder.put("indices.requests.cache.size", new ByteSizeValue(heapSizeBytes)); + IndicesRequestCache cache = new IndicesRequestCache(settingsBuilder.build()); + + Directory dir = newDirectory(); + IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig()); + writer.addDocument(newDoc(0, "foo")); + DirectoryReader reader = OpenSearchDirectoryReader.wrap(DirectoryReader.open(writer), new ShardId("foo", "bar", 1)); + AtomicBoolean indexShard = new AtomicBoolean(true); + + TestEntity entity = new TestEntity(requestCacheStats, indexShard); + Loader loader = new Loader(reader, 0); + System.out.println("On-heap cache size at start = " + requestCacheStats.stats().getMemorySizeInBytes()); + IndicesRequestCache.Key[] keys = new IndicesRequestCache.Key[9]; + for (int i = 0; i < 9; i++) { + TermQueryBuilder termQuery = new TermQueryBuilder("id", String.valueOf(i)); + BytesReference termBytes = XContentHelper.toXContent(termQuery, MediaTypeRegistry.JSON, false); + keys[i] = new IndicesRequestCache.Key(entity, reader.getReaderCacheHelper().getKey(), termBytes); + BytesReference value = cache.getOrCompute(entity, loader, reader, termBytes); + System.out.println("On-heap cache size after " + (i+1) + " queries = " + requestCacheStats.stats().getMemorySizeInBytes()); + System.out.println("Disk cache size after " + (i+1) + " queries = " + requestCacheStats.stats(TierType.DISK).getMemorySizeInBytes()); + } + // attempt to get value from disk cache, the first key should have been evicted + BytesReference firstValue = cache.tieredCacheHandler.get(keys[0]); + System.out.println("Final on-heap cache size = " + requestCacheStats.stats().getMemorySizeInBytes()); // is correctly 920 + //System.out.println("Final self-reported disk size = " + cache.tieredCacheHandler.getDiskWeightBytes()); // is 0, should be 115 + System.out.println("On-heap tier evictions = " + requestCacheStats.stats().getEvictions()); // is correctly 1 + System.out.println("Disk tier hits = " + requestCacheStats.stats(TierType.DISK).getHitCount()); // should be 1, is 0 bc keys not serializable + System.out.println("Disk tier misses = " + requestCacheStats.stats(TierType.DISK).getMissCount()); // should be 9, is 10 bc keys not serializable + //System.out.println("Disk tier self-reported misses = " + cache.tieredCacheHandler.getDiskCachingTier().getMisses()); // should be same as other one + System.out.println("On-heap tier hits = " + requestCacheStats.stats().getHitCount()); // is correctly 0 + System.out.println("On-heap tier misses = " + requestCacheStats.stats().getMissCount()); // is correctly 10 + System.out.println("Disk count = " + cache.tieredCacheHandler.getDiskCachingTier().count()); // should be 1, is 0 + IOUtils.close(reader, writer, dir, cache); + cache.closeDiskTier(); + } + public void testCacheDifferentReaders() throws Exception { IndicesRequestCache cache = new IndicesRequestCache(Settings.EMPTY); AtomicBoolean indexShard = new AtomicBoolean(true); From 66915b6b1a7f79bc8724fc5f3df5ab7ab9bf4e31 Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Fri, 13 Oct 2023 10:07:01 -0700 Subject: [PATCH 11/22] Moved more stuff around for debugging issues --- .../indices/EhcacheDiskCachingTier.java | 61 ++------------ .../indices/EhcacheEventListener.java | 68 +++++++++++++++ .../indices/IndicesRequestCacheTests.java | 83 ++++++++++++++++++- 3 files changed, 159 insertions(+), 53 deletions(-) create mode 100644 server/src/main/java/org/opensearch/indices/EhcacheEventListener.java diff --git a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java index 8fc62fe02bd9a..0ebacae9562c3 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java @@ -27,6 +27,7 @@ import org.ehcache.Cache; import org.opensearch.common.cache.RemovalNotification; import org.opensearch.common.cache.RemovalReason; +import org.opensearch.common.metrics.CounterMetric; import java.util.Collections; @@ -37,7 +38,7 @@ public class EhcacheDiskCachingTier implements CachingTier, RemovalL private final Class keyType; // I think these are needed to pass to newCacheConfigurationBuilder private final Class valueType; - private final String DISK_CACHE_FP = "disk_cache_tier"; // this should probably be defined somewhere else since we need to change security.policy based on its value + public final static String DISK_CACHE_FP = "disk_cache_tier"; // this should probably be defined somewhere else since we need to change security.policy based on its value private RemovalListener removalListener; private StatisticsService statsService; // non functional private ExponentiallyWeightedMovingAverage getTimeMillisEWMA; @@ -46,7 +47,7 @@ public class EhcacheDiskCachingTier implements CachingTier, RemovalL private static final int MAX_WRITE_THREADS = 4; // Max number of threads for the PooledExecutionService which handles writes private static final String cacheAlias = "diskTier"; private final boolean isPersistent; - private int count; // number of entries in cache + private CounterMetric count; // number of entries in cache // private RBMIntKeyLookupStore keystore; // private CacheTierPolicy[] policies; // private IndicesRequestCacheDiskTierPolicy policy; @@ -55,12 +56,12 @@ public EhcacheDiskCachingTier(boolean isPersistent, long maxWeightInBytes, long this.keyType = keyType; this.valueType = valueType; this.isPersistent = isPersistent; - this.count = 0; + this.count = new CounterMetric(); statsService = new DefaultStatisticsService(); // our EhcacheEventListener should receive events every time an entry is changed CacheEventListenerConfigurationBuilder listenerConfig = CacheEventListenerConfigurationBuilder - .newEventListenerConfiguration(new EhcacheEventListener(this, this), + .newEventListenerConfiguration(new EhcacheEventListener(this, this.count), EventType.EVICTED, EventType.EXPIRED, EventType.REMOVED, @@ -157,15 +158,15 @@ public Iterable keys() { @Override public int count() { - return count; + return (int) count.count(); //return (int) getTierStats().getMappings(); } protected void countInc() { - count++; + count.inc(); } protected void countDec() { - count--; + count.dec(); } @Override @@ -228,49 +229,5 @@ public void close() { // This class is used to get the old value from mutating calls to the cache, and it uses those to create a RemovalNotification // It also handles incrementing and decrementing the count for the disk tier, since ehcache's statistics functionality // does not seem to work - private class EhcacheEventListener implements CacheEventListener { // try making these specific, but i dont think itll work - private RemovalListener removalListener; - private EhcacheDiskCachingTier tier; - EhcacheEventListener(RemovalListener removalListener, EhcacheDiskCachingTier tier) { - this.removalListener = removalListener; - this.tier = tier; // needed to handle count changes - } - @Override - public void onEvent(CacheEvent event) { - K key = event.getKey(); - V oldValue = event.getOldValue(); - V newValue = event.getNewValue(); - EventType eventType = event.getType(); - - System.out.println("I am eventing!!"); - - // handle changing count for the disk tier - if (oldValue == null && newValue != null) { - tier.countInc(); - } else if (oldValue != null && newValue == null) { - tier.countDec(); - } - - // handle creating a RemovalReason, unless eventType is CREATED - RemovalReason reason; - switch (eventType) { - case CREATED: - return; - case EVICTED: - reason = RemovalReason.EVICTED; // why is there both RemovalReason.EVICTED and RemovalReason.CAPACITY? - break; - case EXPIRED: - case REMOVED: - reason = RemovalReason.INVALIDATED; - // this is probably fine for EXPIRED. We use cache.remove() to invalidate keys, but this might overlap with RemovalReason.EXPLICIT? - break; - case UPDATED: - reason = RemovalReason.REPLACED; - break; - default: - reason = null; - } - removalListener.onRemoval(new RemovalNotification(key, oldValue, reason)); - } - } + } diff --git a/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java b/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java new file mode 100644 index 0000000000000..ee240467d0af9 --- /dev/null +++ b/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java @@ -0,0 +1,68 @@ +/* + * 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.indices; + +import org.ehcache.event.CacheEvent; +import org.ehcache.event.CacheEventListener; +import org.ehcache.event.EventType; +import org.opensearch.common.cache.RemovalListener; +import org.opensearch.common.cache.RemovalNotification; +import org.opensearch.common.cache.RemovalReason; +import org.opensearch.common.metrics.CounterMetric; +import org.opensearch.indices.EhcacheDiskCachingTier; + +// moved to another file for testing flexibility purposes + +public class EhcacheEventListener implements CacheEventListener { // make it private after debugging + private RemovalListener removalListener; + private CounterMetric counter; + EhcacheEventListener(RemovalListener removalListener, CounterMetric counter) { + this.removalListener = removalListener; + this.counter = counter; // needed to handle count changes + } + @Override + public void onEvent(CacheEvent event) { + K key = event.getKey(); + V oldValue = event.getOldValue(); + V newValue = event.getNewValue(); + EventType eventType = event.getType(); + + System.out.println("I am eventing!!"); + + // handle changing count for the disk tier + if (oldValue == null && newValue != null) { + counter.inc(); + } else if (oldValue != null && newValue == null) { + counter.dec(); + } else { + int j; + } + + // handle creating a RemovalReason, unless eventType is CREATED + RemovalReason reason; + switch (eventType) { + case CREATED: + return; + case EVICTED: + reason = RemovalReason.EVICTED; // why is there both RemovalReason.EVICTED and RemovalReason.CAPACITY? + break; + case EXPIRED: + case REMOVED: + reason = RemovalReason.INVALIDATED; + // this is probably fine for EXPIRED. We use cache.remove() to invalidate keys, but this might overlap with RemovalReason.EXPLICIT? + break; + case UPDATED: + reason = RemovalReason.REPLACED; + break; + default: + reason = null; + } + removalListener.onRemoval(new RemovalNotification(key, oldValue, reason)); + } +} diff --git a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java index e8ad68c67d9c8..375a77815b825 100644 --- a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java +++ b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java @@ -45,10 +45,25 @@ import org.apache.lucene.search.TopDocs; import org.apache.lucene.store.Directory; import org.apache.lucene.util.BytesRef; +import org.ehcache.Cache; +import org.ehcache.PersistentCacheManager; +import org.ehcache.config.builders.CacheConfigurationBuilder; +import org.ehcache.config.builders.CacheEventListenerConfigurationBuilder; +import org.ehcache.config.builders.CacheManagerBuilder; +import org.ehcache.config.builders.PooledExecutionServiceConfigurationBuilder; +import org.ehcache.config.builders.ResourcePoolsBuilder; +import org.ehcache.config.units.MemoryUnit; +import org.ehcache.core.internal.statistics.DefaultStatisticsService; +import org.ehcache.core.spi.service.StatisticsService; import org.ehcache.core.statistics.TierStatistics; +import org.ehcache.event.EventType; +import org.ehcache.impl.config.executor.PooledExecutionServiceConfiguration; import org.opensearch.common.CheckedSupplier; +import org.opensearch.common.cache.RemovalListener; +import org.opensearch.common.cache.RemovalNotification; import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.lucene.index.OpenSearchDirectoryReader; +import org.opensearch.common.metrics.CounterMetric; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.io.IOUtils; import org.opensearch.core.common.bytes.AbstractBytesReference; @@ -62,6 +77,7 @@ import org.opensearch.test.OpenSearchTestCase; import java.io.IOException; +import java.io.Serializable; import java.util.Arrays; import java.util.concurrent.atomic.AtomicBoolean; @@ -150,6 +166,71 @@ public void testAddDirectToEhcache() throws Exception { cache.closeDiskTier(); } + public void testSimpleEhcache() throws Exception { + // for debug only, delete + CounterMetric count = new CounterMetric(); + String cacheAlias = "dummy"; + + class DummyRemovalListener implements RemovalListener { + public DummyRemovalListener() { } + @Override + public void onRemoval(RemovalNotification notification) { + System.out.println(":)"); + } + } + + CacheEventListenerConfigurationBuilder listenerConfig = CacheEventListenerConfigurationBuilder + .newEventListenerConfiguration(new EhcacheEventListener(new DummyRemovalListener(), count), + EventType.EVICTED, + EventType.EXPIRED, + EventType.REMOVED, + EventType.UPDATED, + EventType.CREATED) + .ordered().asynchronous(); // ordered() has some performance penalty as compared to unordered(), we can also use synchronous() + + StatisticsService statsService = new DefaultStatisticsService(); + + PooledExecutionServiceConfiguration threadConfig = PooledExecutionServiceConfigurationBuilder.newPooledExecutionServiceConfigurationBuilder() + .defaultPool("default", 0, 4) + .build(); + + PersistentCacheManager cacheManager = CacheManagerBuilder.newCacheManagerBuilder() + .using(statsService) // https://stackoverflow.com/questions/40453859/how-to-get-ehcache-3-1-statistics + .using(threadConfig) + .with(CacheManagerBuilder.persistence(EhcacheDiskCachingTier.DISK_CACHE_FP)) + .withCache(cacheAlias, CacheConfigurationBuilder.newCacheConfigurationBuilder( + IndicesRequestCache.Key.class, String.class, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(10, MemoryUnit.MB, false)) + .withService(listenerConfig) // stackoverflow shows .add(), but IDE says this is deprecated. idk + ).build(true); + Cache cache = cacheManager.getCache(cacheAlias, IndicesRequestCache.Key.class, String.class); + + Directory dir = newDirectory(); + IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig()); + writer.addDocument(newDoc(0, "foo")); + DirectoryReader reader = OpenSearchDirectoryReader.wrap(DirectoryReader.open(writer), new ShardId("foo", "bar", 1)); + AtomicBoolean indexShard = new AtomicBoolean(true); + ShardRequestCache requestCacheStats = new ShardRequestCache(); + TestEntity entity = new TestEntity(requestCacheStats, indexShard); + Loader loader = new Loader(reader, 0); + System.out.println("On-heap cache size at start = " + requestCacheStats.stats().getMemorySizeInBytes()); + IndicesRequestCache.Key[] keys = new IndicesRequestCache.Key[9]; + TermQueryBuilder termQuery = new TermQueryBuilder("id", "0"); + BytesReference termBytes = XContentHelper.toXContent(termQuery, MediaTypeRegistry.JSON, false); + IndicesRequestCache.Key key = new IndicesRequestCache.Key(entity, reader.getReaderCacheHelper().getKey(), termBytes); + + cache.put(key, "blorp"); + System.out.println("Counter value = " + count.count()); + String res = cache.get(key); + System.out.println("Got result " + res); + + System.out.println("Counter value = " + count.count()); + //System.out.println("Hits = " + statsService.getCacheStatistics(cacheAlias).getTierStatistics().get("Disk").getHits()); + + cacheManager.removeCache(cacheAlias); + cacheManager.close(); + IOUtils.close(reader, writer, dir); + } + public void testSpillover() throws Exception { // fill the on-heap cache until we spill over ShardRequestCache requestCacheStats = new ShardRequestCache(); @@ -534,7 +615,7 @@ public void testEqualsKey() throws IOException { assertNotEquals(key1, key5); } - private class TestBytesReference extends AbstractBytesReference { + private class TestBytesReference extends AbstractBytesReference implements Serializable { int dummyValue; From f985e6371bccd08b2668855e15033dd5188764a3 Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Fri, 13 Oct 2023 16:39:04 -0700 Subject: [PATCH 12/22] fixed various serialization issues relating to silent ehcache put() and get() failures --- server/build.gradle | 2 +- .../indices/DummySerializableKey.java | 50 +++++++++++++ .../indices/EhcacheEventListener.java | 2 +- .../indices/IndicesRequestCacheTests.java | 74 ++++++++++++++----- 4 files changed, 107 insertions(+), 21 deletions(-) create mode 100644 server/src/main/java/org/opensearch/indices/DummySerializableKey.java diff --git a/server/build.gradle b/server/build.gradle index 77cc411677ac5..dc3fa97ea4ca6 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -159,7 +159,7 @@ dependencies { api "jakarta.annotation:jakarta.annotation-api:${versions.jakarta_annotation}" // ehcache - api "org.ehcache:ehcache:3.10.8" + api "org.ehcache:ehcache:3.10.8" // using 3.3.1 rather than 3.10.8 to test if dependency conflicts are causing my issues api "org.slf4j:slf4j-api:1.7.36" testImplementation(project(":test:framework")) { diff --git a/server/src/main/java/org/opensearch/indices/DummySerializableKey.java b/server/src/main/java/org/opensearch/indices/DummySerializableKey.java new file mode 100644 index 0000000000000..7f2888f6e65f7 --- /dev/null +++ b/server/src/main/java/org/opensearch/indices/DummySerializableKey.java @@ -0,0 +1,50 @@ +/* + * 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.indices; + +import java.io.Serializable; +import java.util.Objects; + +public class DummySerializableKey implements Serializable { + private Integer i; + private String s; + public DummySerializableKey(Integer i, String s) { + this.i = i; + this.s = s; + } + + public int getI() { + return i; + } + public String getS() { + return s; + } + @Override + public boolean equals(Object o) { + if (o == this) { + return true; + } + if (!(o instanceof DummySerializableKey)) { + return false; + } + DummySerializableKey other = (DummySerializableKey) o; + return Objects.equals(this.i, other.i) && this.s.equals(other.s); + } + @Override + public final int hashCode() { + int result = 11; + if (i != null) { + result = 31 * result + i.hashCode(); + } + if (s != null) { + result = 31 * result + s.hashCode(); + } + return result; + } +} diff --git a/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java b/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java index ee240467d0af9..c6d2790dda95f 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java @@ -41,7 +41,7 @@ public void onEvent(CacheEvent event) { } else if (oldValue != null && newValue == null) { counter.dec(); } else { - int j; + int j; // breakpoint } // handle creating a RemovalReason, unless eventType is CREATED diff --git a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java index 375a77815b825..8cdf5635439af 100644 --- a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java +++ b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java @@ -78,7 +78,9 @@ import java.io.IOException; import java.io.Serializable; +import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; public class IndicesRequestCacheTests extends OpenSearchTestCase { @@ -194,17 +196,58 @@ public void onRemoval(RemovalNotification notification) { .defaultPool("default", 0, 4) .build(); - PersistentCacheManager cacheManager = CacheManagerBuilder.newCacheManagerBuilder() - .using(statsService) // https://stackoverflow.com/questions/40453859/how-to-get-ehcache-3-1-statistics - .using(threadConfig) - .with(CacheManagerBuilder.persistence(EhcacheDiskCachingTier.DISK_CACHE_FP)) - .withCache(cacheAlias, CacheConfigurationBuilder.newCacheConfigurationBuilder( - IndicesRequestCache.Key.class, String.class, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(10, MemoryUnit.MB, false)) - .withService(listenerConfig) // stackoverflow shows .add(), but IDE says this is deprecated. idk - ).build(true); - Cache cache = cacheManager.getCache(cacheAlias, IndicesRequestCache.Key.class, String.class); + PersistentCacheManager cacheManager; + + boolean doIntCache = false; + + if (doIntCache) { + cacheManager = CacheManagerBuilder.newCacheManagerBuilder() + .using(statsService) // https://stackoverflow.com/questions/40453859/how-to-get-ehcache-3-1-statistics + .using(threadConfig) + .with(CacheManagerBuilder.persistence(EhcacheDiskCachingTier.DISK_CACHE_FP)) + .withCache(cacheAlias, CacheConfigurationBuilder.newCacheConfigurationBuilder( + Integer.class, String.class, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(10, MemoryUnit.MB, false)) + .withService(listenerConfig) + ).build(true); + Cache integerCache = cacheManager.getCache(cacheAlias, Integer.class, String.class); + + integerCache.put(0, "blorp"); + System.out.println("Counter value = " + count.count()); + String res = integerCache.get(0); + System.out.println("Got result " + res); + System.out.println("Counter value = " + count.count()); + } else { + cacheManager = CacheManagerBuilder.newCacheManagerBuilder() + .using(statsService) // https://stackoverflow.com/questions/40453859/how-to-get-ehcache-3-1-statistics + .using(threadConfig) + .with(CacheManagerBuilder.persistence(EhcacheDiskCachingTier.DISK_CACHE_FP)) + .withCache(cacheAlias, CacheConfigurationBuilder.newCacheConfigurationBuilder( + DummySerializableKey.class, String.class, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(10, MemoryUnit.MB, false)) + .withService(listenerConfig) + ).build(true); + Cache cache = cacheManager.getCache(cacheAlias, DummySerializableKey.class, String.class); + + DummySerializableKey key = new DummySerializableKey(Integer.valueOf(0), "blah"); + cache.put(key, "blorp"); + System.out.println("Counter value = " + count.count()); + String res = cache.get(key); + System.out.println("Got result " + res); + System.out.println("Counter value = " + count.count()); + TierStatistics ts = statsService.getCacheStatistics(cacheAlias).getTierStatistics().get("Disk"); + System.out.println("self-reported count = " + ts.getMappings()); + System.out.println("self-reported misses = " + ts.getMisses()); + System.out.println("self-reported hits = " + ts.getHits()); + + List> foos = new ArrayList<>(); + for(Cache.Entry entry : cache) { + foos.add(entry); + } + int j = 0; + j++; + System.out.println(j); + } - Directory dir = newDirectory(); + /*Directory dir = newDirectory(); IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig()); writer.addDocument(newDoc(0, "foo")); DirectoryReader reader = OpenSearchDirectoryReader.wrap(DirectoryReader.open(writer), new ShardId("foo", "bar", 1)); @@ -216,19 +259,12 @@ public void onRemoval(RemovalNotification notification) { IndicesRequestCache.Key[] keys = new IndicesRequestCache.Key[9]; TermQueryBuilder termQuery = new TermQueryBuilder("id", "0"); BytesReference termBytes = XContentHelper.toXContent(termQuery, MediaTypeRegistry.JSON, false); - IndicesRequestCache.Key key = new IndicesRequestCache.Key(entity, reader.getReaderCacheHelper().getKey(), termBytes); - - cache.put(key, "blorp"); - System.out.println("Counter value = " + count.count()); - String res = cache.get(key); - System.out.println("Got result " + res); + IndicesRequestCache.Key key = new IndicesRequestCache.Key(entity, reader.getReaderCacheHelper().getKey(), termBytes);*/ - System.out.println("Counter value = " + count.count()); - //System.out.println("Hits = " + statsService.getCacheStatistics(cacheAlias).getTierStatistics().get("Disk").getHits()); cacheManager.removeCache(cacheAlias); cacheManager.close(); - IOUtils.close(reader, writer, dir); + //IOUtils.close(reader, writer, dir); } public void testSpillover() throws Exception { From ae300e9c3c789d8adf7605a2a18c0a9528352cf9 Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Mon, 16 Oct 2023 10:03:50 -0700 Subject: [PATCH 13/22] Cleanup --- ...tJavaSerializer-ObjectStreamClassIndex.bin | Bin 3081 -> 0 bytes .../ehcache-disk-store.data | Bin 16384 -> 0 bytes .../ehcache-disk-store.index | Bin 473 -> 0 bytes .../ehcache-disk-store.meta | 4 -- ...tJavaSerializer-ObjectStreamClassIndex.bin | Bin 1367 -> 0 bytes .../indices/EhcacheDiskCachingTier.java | 55 +++--------------- .../indices/IndicesRequestCache.java | 5 +- .../TieredCacheSpilloverStrategyHandler.java | 2 +- .../indices/IndicesRequestCacheTests.java | 7 ++- 9 files changed, 17 insertions(+), 56 deletions(-) delete mode 100644 server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/key-serializer/holder-0-CompactJavaSerializer-ObjectStreamClassIndex.bin delete mode 100644 server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.data delete mode 100644 server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.index delete mode 100644 server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.meta delete mode 100644 server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/value-serializer/holder-0-CompactJavaSerializer-ObjectStreamClassIndex.bin diff --git a/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/key-serializer/holder-0-CompactJavaSerializer-ObjectStreamClassIndex.bin b/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/key-serializer/holder-0-CompactJavaSerializer-ObjectStreamClassIndex.bin deleted file mode 100644 index a8f701c5f90c8437b80ab6beaadff9b5cc26b90b..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 3081 zcmeHJU1%It6u#L_()?H(;~!|-h^)4ebOwzHiJP`I+k~!e65NeN%+t-x+1*KI=1%Xu z)9h9eMQjBT1mlCEMMS9Dh%Y|G2T?Evl-PxZeh9E|Ka36Me z?ws?TbH4L;SAHdJk_@Dn7f?$+LQm5iaEH3i8Q_Oz<`=+{Imv-~DVGY7_H8H$PLhll z3@BREDuUs7folZ7g(D>R78sL`3*%IPos(2T76K+5$#^+28wM_Pj(pzoY3FhnBe67T za};8Rv=^Cc1DB*fgV1Ib+7y9fHXxfC@PrmkmI8twbY^gaGol&-l@9ZbSx$Z7fRBNo zI~DB<>(5_&^P%LG7-`9nxJLs?5}5^z-VCMVnilh|kaN^AQ?Kb)Lqpo;@iXtl8udnyMb*}$#oW#?lTR_1>{3$9H)8vNgQ?BUbW%{39C zxHcneo4Z-HmdI5MRCwBhRP7CU6|OmT5!{x}*Wkk81D98JjeQ*>L}#D}2|4oj2#GNJ zhBL1`@oN8JOvWVXav8NJu@)IGadJ@Q#85eD^v-40qA;?6{9$fo1|Rc}zc(=O)xs}2 zp{=6qTauWYXH;vOP1=bZDpZpD;C3x0rkS6o4p*x1;saN6Pkq*{Na5trwjq@6>i&0@ zKREv6(-qyJEp(5~3yjy2kA)CobD?gLGhcNoMxP3gW`D%!v8RNpbM#|)8(1?-KAm@= z5p3|vN1uG?vG=14lcNt~pPeGlV414#9@TgMpKZYbCpT&8UFsK%G|q+s@ZKNZe)sq1 z&W%)3beg)MN>NW;ZaVblfuCJ@ZD05D_b=7R=fqIm#RBjJP;M0s9GQ*<0)h2!7EXmA zWJ=BA#G_E2T$4YqE%#oEkvPs(Y!OG6B^l9Uq>h3^L~EOyp=hiLt(nY_8*%*OY}SW} zFNvYCC@zd5m~Lo+4+;)S3JgkFEuM(v5ur_oE6M%WJ7F;nIK@1V`R06Cf-uL`cNX%1 z!`A}UlB(nevEn*7>8lrXin%WCCY-*W}d{TR6E(D>}@W zW-{7us_Z9976c09;K>rXdiJ)VN)zsj#2Xi2P1gS6r^S~?|G?d$n*HWTHa%05aGw=6 z-1FPTMhRUDHHmMo^&Pn*xp!@Y#4NV-l5*|aOG5`nmsT&wNh=a)(NKUSw^k;a!ZK@; zNbqhOBz4>=5P}v^uD3xB*SF&rl5?aB=Yq9}+t5OZ{c}ws(TfQe)K)^sTSOQH909?u zd5!z1r(vcl${RBjuFMlsU}*~$??AkUA>!VD98A}n))3Gpfw8MAE?Y7 zaIdY&y;`wv(iS^lyq!l4VrX{o=2-2Vs6k?r_l=ERyL(hMaD7JY;^a;}4>s4LonO9j z@9=jo#7M_DY0oq6QJKaYkK;pS`e(=w9?hv?Gr< h?FOkX4`h+qO()C_n`Cx-`sAd#uQOi*m4YBB{R#TT{U-nb diff --git a/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.data b/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.data deleted file mode 100644 index 294f4016d05bdd696670c4840f1f36a71f9239de..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 16384 zcmeIuF#!Mo0K%a4Pi+hzh(KY$fB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM q7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjMyaxtA00031 diff --git a/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.index b/server/disk_cache_tier/file/diskTier_8a0816fa8975d40fe56fbdb0d44b485392d49431/offheap-disk-store/ehcache-disk-store.index deleted file mode 100644 index 05ab5af78b038e9fc54d3d0ee751c5b0e700edd0..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 473 zcmZ4UmVvd3fr0S?5O=G-e=QsC80^Wwz#!o1;{&3hfCWSgIQqJ{f*1%nkT@^ z#avXJ1^f=G@ws{nJ>wdMEG!31>#AsATPr9@wP>O5G0#Vfhf!cfs5I9Gg@;y!2Y8Wb zY`BIQtYIWIH&QL<=Hn>9kuzV0K8>uz8OT&&m?w-5?Db{PKxH8B5Vf6B+dATQAG6&d zoaua8YDYjnGBdHwj7~>jhD%}1D<(8YL4%N;F8bx#gSXGlmew;cM0Q=LUHZZ-gC6FX*yQj9Z%~UPtb_KdQ9ipg?#44rRT3wHBSMwWm zaq;-K)qUr_X8<}4j8P!x|1FRn=dq&m`1+IlB$ZKt1A%0X3f)Di+6qo2omfdH{q6$k zEorNUlpl3#96C0yy($#GwtjUA-J_R implements CachingTier, RemovalListener { private final PersistentCacheManager cacheManager; - public final Cache cache; // make private after debug + private final Cache cache; // make private after debug private final Class keyType; // I think these are needed to pass to newCacheConfigurationBuilder private final Class valueType; @@ -48,6 +48,7 @@ public class EhcacheDiskCachingTier implements CachingTier, RemovalL private static final String cacheAlias = "diskTier"; private final boolean isPersistent; private CounterMetric count; // number of entries in cache + private EhcacheEventListener listener; // private RBMIntKeyLookupStore keystore; // private CacheTierPolicy[] policies; // private IndicesRequestCacheDiskTierPolicy policy; @@ -57,17 +58,18 @@ public EhcacheDiskCachingTier(boolean isPersistent, long maxWeightInBytes, long this.valueType = valueType; this.isPersistent = isPersistent; this.count = new CounterMetric(); + this.listener = new EhcacheEventListener(this, this.count); statsService = new DefaultStatisticsService(); // our EhcacheEventListener should receive events every time an entry is changed CacheEventListenerConfigurationBuilder listenerConfig = CacheEventListenerConfigurationBuilder - .newEventListenerConfiguration(new EhcacheEventListener(this, this.count), + .newEventListenerConfiguration(listener, EventType.EVICTED, EventType.EXPIRED, EventType.REMOVED, EventType.UPDATED, - EventType.CREATED); - //.ordered().asynchronous(); // ordered() has some performance penalty as compared to unordered(), we can also use synchronous() + EventType.CREATED) + .ordered().asynchronous(); // ordered() has some performance penalty as compared to unordered(), we can also use synchronous() PooledExecutionServiceConfiguration threadConfig = PooledExecutionServiceConfigurationBuilder.newPooledExecutionServiceConfigurationBuilder() .defaultPool("default", MIN_WRITE_THREADS, MAX_WRITE_THREADS) @@ -79,7 +81,7 @@ public EhcacheDiskCachingTier(boolean isPersistent, long maxWeightInBytes, long .with(CacheManagerBuilder.persistence(DISK_CACHE_FP)) .withCache(cacheAlias, CacheConfigurationBuilder.newCacheConfigurationBuilder( keyType, valueType, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(maxWeightInBytes, MemoryUnit.B, isPersistent)) - .withService(listenerConfig) // stackoverflow shows .add(), but IDE says this is deprecated. idk + .withService(listenerConfig) ).build(true); this.cache = cacheManager.getCache(cacheAlias, keyType, valueType); this.getTimeMillisEWMA = new ExponentiallyWeightedMovingAverage(GET_TIME_EWMA_ALPHA, 10); @@ -91,7 +93,7 @@ public EhcacheDiskCachingTier(boolean isPersistent, long maxWeightInBytes, long @Override public V get(K key) { - // do we need to do the future stuff? I don't think so? + // I don't think we need to do the future stuff as the cache is threadsafe // if (keystore.contains(key.hashCode()) { long now = System.nanoTime(); @@ -101,7 +103,6 @@ public V get(K key) { return value; // } // return null; - } @Override @@ -111,7 +112,6 @@ public void put(K key, V value) { // CheckDataResult policyResult = policy.checkData(value) // if (policyResult.isAccepted()) { cache.put(key, value); - //count++; // keystore.add(key.hashCode()); // else { do something with policyResult.deniedReason()? } // } @@ -129,7 +129,6 @@ public void invalidate(K key) { // if (keystore.contains(key.hashCode()) { cache.remove(key); - //count--; // keystore.remove(key.hashCode()); // } } @@ -159,7 +158,6 @@ public Iterable keys() { @Override public int count() { return (int) count.count(); - //return (int) getTierStats().getMappings(); } protected void countInc() { @@ -183,35 +181,6 @@ public double getTimeMillisEWMA() { return getTimeMillisEWMA.getAverage(); } - // these aren't really needed, ShardRequestCache handles it - // Also, it seems that ehcache doesn't have functioning statistics anyway! - - /*public TierStatistics getTierStats() { - return statsService.getCacheStatistics(cacheAlias).getTierStatistics().get("Disk"); - } - - public long getHits() { - return getTierStats().getHits(); - } - - public long getMisses() { - return getTierStats().getMisses(); - } - - public long getWeightBytes() { - return getTierStats().getOccupiedByteSize(); - } - - public long getEvictions() { - return getTierStats().getEvictions(); - } - - public double getHitRatio() { - TierStatistics ts = getTierStats(); - long hits = ts.getHits(); - return hits / (hits + ts.getMisses()); - }*/ - public boolean isPersistent() { return isPersistent; } @@ -222,12 +191,4 @@ public void close() { cacheManager.removeCache(cacheAlias); cacheManager.close(); } - - - // See https://stackoverflow.com/questions/45827753/listenerobject-not-found-in-imports-for-ehcache-3 for API reference - // it's not actually documented by ehcache :( - // This class is used to get the old value from mutating calls to the cache, and it uses those to create a RemovalNotification - // It also handles incrementing and decrementing the count for the disk tier, since ehcache's statistics functionality - // does not seem to work - } diff --git a/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java b/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java index 9a475be609a1a..80565f3fe127e 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java +++ b/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java @@ -46,7 +46,6 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.concurrent.ConcurrentCollections; -import org.opensearch.core.common.bytes.BytesArray; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.common.unit.ByteSizeValue; @@ -107,8 +106,8 @@ public final class IndicesRequestCache implements TieredCacheEventListener cache; - //private final TieredCacheHandler tieredCacheHandler; // made public TieredCacheSpilloverStrategyHandler for testing - public final TieredCacheSpilloverStrategyHandler tieredCacheHandler; + //private final TieredCacheHandler tieredCacheHandler; + public final TieredCacheSpilloverStrategyHandler tieredCacheHandler; // Change this back after done debugging serialization issues IndicesRequestCache(Settings settings) { this.size = INDICES_CACHE_QUERY_SIZE.get(settings); this.expire = INDICES_CACHE_QUERY_EXPIRE.exists(settings) ? INDICES_CACHE_QUERY_EXPIRE.get(settings) : null; diff --git a/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java b/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java index 5657e3519f2ca..589c7dcfd9887 100644 --- a/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java +++ b/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java @@ -36,7 +36,7 @@ public class TieredCacheSpilloverStrategyHandler implements TieredCacheHan private TieredCacheSpilloverStrategyHandler( OnHeapCachingTier onHeapCachingTier, EhcacheDiskCachingTier diskCachingTier, - // changed to EhcacheDiskCachingTier from CachingTier, to enable close() method, which is needed for tests. Implement close() in CachingTier or DiskCachingTier or something? + // changed to EhcacheDiskCachingTier from CachingTier, to enable close() method, which is needed for tests. Implement close() in CachingTier or DiskCachingTier? TieredCacheEventListener tieredCacheEventListener ) { this.onHeapCachingTier = Objects.requireNonNull(onHeapCachingTier); diff --git a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java index 8cdf5635439af..14cca8ef60681 100644 --- a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java +++ b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java @@ -142,6 +142,9 @@ public void testBasicOperationsCache() throws Exception { } public void testAddDirectToEhcache() throws Exception { + // this test is for debugging serialization issues and can eventually be removed + // Put breakpoints at line 260 of AbstractOffHeapStore to catch serialization errors + // that would otherwise fail silently ShardRequestCache requestCacheStats = new ShardRequestCache(); Settings.Builder settingsBuilder = Settings.builder(); long heapSizeBytes = 1000; @@ -162,7 +165,9 @@ public void testAddDirectToEhcache() throws Exception { IndicesRequestCache.Key key = new IndicesRequestCache.Key(entity, reader.getReaderCacheHelper().getKey(), termBytes); TestBytesReference value = new TestBytesReference(124); - cache.tieredCacheHandler.getDiskCachingTier().cache.put(key, value); + cache.tieredCacheHandler.getDiskCachingTier().put(key, value); + + System.out.println("Size: " + cache.tieredCacheHandler.getDiskCachingTier().count()); IOUtils.close(reader, writer, dir, cache); cache.closeDiskTier(); From 7a7b41fa951c3312b21a4bf1aa3fa7f0be1f237a Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Mon, 16 Oct 2023 12:00:33 -0700 Subject: [PATCH 14/22] More cleanup --- .../java/org/opensearch/common/metrics/CounterMetric.java | 1 + .../opensearch/index/cache/request/ShardRequestCache.java | 3 ++- .../org/opensearch/indices/IndicesRequestCacheTests.java | 6 ++++-- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/opensearch/common/metrics/CounterMetric.java b/server/src/main/java/org/opensearch/common/metrics/CounterMetric.java index cb181840406a5..33fc5e32e9c60 100644 --- a/server/src/main/java/org/opensearch/common/metrics/CounterMetric.java +++ b/server/src/main/java/org/opensearch/common/metrics/CounterMetric.java @@ -32,6 +32,7 @@ package org.opensearch.common.metrics; +import java.io.Serializable; import java.util.concurrent.atomic.LongAdder; /** diff --git a/server/src/main/java/org/opensearch/index/cache/request/ShardRequestCache.java b/server/src/main/java/org/opensearch/index/cache/request/ShardRequestCache.java index f09d2504680c7..3194aee757fc4 100644 --- a/server/src/main/java/org/opensearch/index/cache/request/ShardRequestCache.java +++ b/server/src/main/java/org/opensearch/index/cache/request/ShardRequestCache.java @@ -37,6 +37,7 @@ import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.indices.TierType; +import java.io.Serializable; import java.util.EnumMap; /** @@ -113,7 +114,7 @@ public void onRemoval(Accountable key, BytesReference value, boolean evicted, Ti statsHolder.get(tierType).totalMetric.dec(dec); } - static class StatsHolder { + static class StatsHolder implements Serializable { final CounterMetric evictionsMetric = new CounterMetric(); final CounterMetric totalMetric = new CounterMetric(); diff --git a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java index 14cca8ef60681..5636268d78eb1 100644 --- a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java +++ b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java @@ -67,6 +67,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.util.io.IOUtils; import org.opensearch.core.common.bytes.AbstractBytesReference; +import org.opensearch.core.common.bytes.BytesArray; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.common.unit.ByteSizeValue; import org.opensearch.core.index.shard.ShardId; @@ -164,7 +165,8 @@ public void testAddDirectToEhcache() throws Exception { BytesReference termBytes = XContentHelper.toXContent(termQuery, MediaTypeRegistry.JSON, false); IndicesRequestCache.Key key = new IndicesRequestCache.Key(entity, reader.getReaderCacheHelper().getKey(), termBytes); - TestBytesReference value = new TestBytesReference(124); + //TestBytesReference value = new TestBytesReference(124); + BytesReference value = new BytesArray(new byte[]{0}); cache.tieredCacheHandler.getDiskCachingTier().put(key, value); System.out.println("Size: " + cache.tieredCacheHandler.getDiskCachingTier().count()); @@ -707,7 +709,7 @@ public boolean isFragment() { } } - private class TestEntity extends AbstractIndexShardCacheEntity { + private class TestEntity extends AbstractIndexShardCacheEntity implements Serializable { private final AtomicBoolean standInForIndexShard; private final ShardRequestCache shardRequestCache; From 990ba9cdb73fca678aca134e00cbfdaf959fc2c9 Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Mon, 16 Oct 2023 16:25:06 -0700 Subject: [PATCH 15/22] Part 1 of modifying ehcache to take serializable Key --- .../core/common/bytes/BytesReference.java | 2 +- .../org/opensearch/indices/CachingTier.java | 8 ++- .../opensearch/indices/DiskCachingTier.java | 5 +- .../indices/EhcacheDiskCachingTier.java | 59 +++++++++++++------ .../indices/EhcacheEventListener.java | 31 ++++++---- .../org/opensearch/indices/EhcacheKey.java | 54 +++++++++++++++++ .../TieredCacheSpilloverStrategyHandler.java | 9 +-- 7 files changed, 129 insertions(+), 39 deletions(-) create mode 100644 server/src/main/java/org/opensearch/indices/EhcacheKey.java diff --git a/libs/core/src/main/java/org/opensearch/core/common/bytes/BytesReference.java b/libs/core/src/main/java/org/opensearch/core/common/bytes/BytesReference.java index 60613d4e2a997..e07aca89339b9 100644 --- a/libs/core/src/main/java/org/opensearch/core/common/bytes/BytesReference.java +++ b/libs/core/src/main/java/org/opensearch/core/common/bytes/BytesReference.java @@ -55,7 +55,7 @@ * @opensearch.api */ @PublicApi(since = "1.0.0") -public interface BytesReference extends Comparable, ToXContentFragment, Serializable { // another lie! +public interface BytesReference extends Comparable, ToXContentFragment { /** * Convert an {@link XContentBuilder} into a BytesReference. This method closes the builder, diff --git a/server/src/main/java/org/opensearch/indices/CachingTier.java b/server/src/main/java/org/opensearch/indices/CachingTier.java index 85596929cfd6b..8c0dc0936f9dc 100644 --- a/server/src/main/java/org/opensearch/indices/CachingTier.java +++ b/server/src/main/java/org/opensearch/indices/CachingTier.java @@ -10,6 +10,8 @@ import org.opensearch.common.cache.RemovalListener; +import java.io.IOException; + /** * asdsadssa * @param @@ -17,13 +19,13 @@ */ public interface CachingTier { - V get(K key); + V get(K key) throws IOException; - void put(K key, V value); + void put(K key, V value) throws IOException; V computeIfAbsent(K key, TieredCacheLoader loader) throws Exception; - void invalidate(K key); + void invalidate(K key) throws IOException; V compute(K key, TieredCacheLoader loader) throws Exception; diff --git a/server/src/main/java/org/opensearch/indices/DiskCachingTier.java b/server/src/main/java/org/opensearch/indices/DiskCachingTier.java index efd9a459cd338..de88c44cc1e8f 100644 --- a/server/src/main/java/org/opensearch/indices/DiskCachingTier.java +++ b/server/src/main/java/org/opensearch/indices/DiskCachingTier.java @@ -9,5 +9,8 @@ package org.opensearch.indices; public interface DiskCachingTier extends CachingTier { - + /** + * Closes the disk tier. + */ + void close(); } diff --git a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java index 2142b8f3b0930..4a34fea847981 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java @@ -27,16 +27,24 @@ import org.ehcache.Cache; import org.opensearch.common.cache.RemovalNotification; import org.opensearch.common.cache.RemovalReason; +import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.metrics.CounterMetric; - +import org.opensearch.core.common.bytes.BytesReference; +import org.opensearch.core.common.io.stream.BytesStreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.core.common.io.stream.Writeable; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; import java.util.Collections; -public class EhcacheDiskCachingTier implements CachingTier, RemovalListener { +public class EhcacheDiskCachingTier implements DiskCachingTier, RemovalListener { private final PersistentCacheManager cacheManager; - private final Cache cache; // make private after debug - - private final Class keyType; // I think these are needed to pass to newCacheConfigurationBuilder + private final Cache cache; + private final Class keyType; // These are needed to pass to newCacheConfigurationBuilder + //private final Class> ehcacheKeyType; private final Class valueType; public final static String DISK_CACHE_FP = "disk_cache_tier"; // this should probably be defined somewhere else since we need to change security.policy based on its value private RemovalListener removalListener; @@ -48,17 +56,25 @@ public class EhcacheDiskCachingTier implements CachingTier, RemovalL private static final String cacheAlias = "diskTier"; private final boolean isPersistent; private CounterMetric count; // number of entries in cache - private EhcacheEventListener listener; + private final EhcacheEventListener listener; // private RBMIntKeyLookupStore keystore; // private CacheTierPolicy[] policies; // private IndicesRequestCacheDiskTierPolicy policy; - public EhcacheDiskCachingTier(boolean isPersistent, long maxWeightInBytes, long maxKeystoreWeightInBytes, Class keyType, Class valueType) { + public EhcacheDiskCachingTier( + boolean isPersistent, + long maxWeightInBytes, + long maxKeystoreWeightInBytes, + Class keyType, + //Class> ehcacheKeyType, + Class valueType + ) { + this.isPersistent = isPersistent; this.keyType = keyType; + //this.ehcacheKeyType = ehcacheKeyType; this.valueType = valueType; - this.isPersistent = isPersistent; this.count = new CounterMetric(); - this.listener = new EhcacheEventListener(this, this.count); + this.listener = new EhcacheEventListener(this, this); statsService = new DefaultStatisticsService(); // our EhcacheEventListener should receive events every time an entry is changed @@ -80,10 +96,10 @@ public EhcacheDiskCachingTier(boolean isPersistent, long maxWeightInBytes, long .using(threadConfig) .with(CacheManagerBuilder.persistence(DISK_CACHE_FP)) .withCache(cacheAlias, CacheConfigurationBuilder.newCacheConfigurationBuilder( - keyType, valueType, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(maxWeightInBytes, MemoryUnit.B, isPersistent)) + EhcacheKey.class, valueType, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(maxWeightInBytes, MemoryUnit.B, isPersistent)) .withService(listenerConfig) ).build(true); - this.cache = cacheManager.getCache(cacheAlias, keyType, valueType); + this.cache = cacheManager.getCache(cacheAlias, EhcacheKey.class, valueType); this.getTimeMillisEWMA = new ExponentiallyWeightedMovingAverage(GET_TIME_EWMA_ALPHA, 10); // this.keystore = new RBMIntKeyLookupStore((int) Math.pow(2, 28), maxKeystoreWeightInBytes); @@ -92,12 +108,12 @@ public EhcacheDiskCachingTier(boolean isPersistent, long maxWeightInBytes, long } @Override - public V get(K key) { + public V get(K key) throws IOException { // I don't think we need to do the future stuff as the cache is threadsafe // if (keystore.contains(key.hashCode()) { long now = System.nanoTime(); - V value = cache.get(key); + V value = cache.get(new EhcacheKey(key)); double tookTimeMillis = ((double) (System.nanoTime() - now)) / 1000000; getTimeMillisEWMA.addValue(tookTimeMillis); return value; @@ -106,12 +122,12 @@ public V get(K key) { } @Override - public void put(K key, V value) { + public void put(K key, V value) throws IOException { // No need to get old value, this is handled by EhcacheEventListener. // CheckDataResult policyResult = policy.checkData(value) // if (policyResult.isAccepted()) { - cache.put(key, value); + cache.put(new EhcacheKey(key), value); // keystore.add(key.hashCode()); // else { do something with policyResult.deniedReason()? } // } @@ -123,12 +139,12 @@ public V computeIfAbsent(K key, TieredCacheLoader loader) throws Exception } @Override - public void invalidate(K key) { + public void invalidate(K key) throws IOException { // keep keystore check to avoid unneeded disk seek // RemovalNotification is handled by EhcacheEventListener // if (keystore.contains(key.hashCode()) { - cache.remove(key); + cache.remove(new EhcacheKey(key)); // keystore.remove(key.hashCode()); // } } @@ -185,6 +201,15 @@ public boolean isPersistent() { return isPersistent; } + public K convertEhcacheKeyToOriginal(EhcacheKey eKey) throws IOException { + BytesStreamInput is = new BytesStreamInput(); + byte[] bytes = eKey.getBytes(); + is.readBytes(bytes, 0, bytes.length); + // we somehow have to use the Reader thing in the Writeable interface + // otherwise its not generic + } + + @Override public void close() { // Call this method after each test, otherwise the directory will stay locked and you won't be able to // initialize another IndicesRequestCache (for example in the next test that runs) diff --git a/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java b/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java index c6d2790dda95f..6c355dba025e1 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java @@ -15,33 +15,34 @@ import org.opensearch.common.cache.RemovalNotification; import org.opensearch.common.cache.RemovalReason; import org.opensearch.common.metrics.CounterMetric; +import org.opensearch.core.common.io.stream.BytesStreamInput; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.Writeable; import org.opensearch.indices.EhcacheDiskCachingTier; // moved to another file for testing flexibility purposes -public class EhcacheEventListener implements CacheEventListener { // make it private after debugging +public class EhcacheEventListener implements CacheEventListener { + // Receives key-value pairs (BytesReference, BytesReference), but must transform into (Key, BytesReference) + // to send removal notifications private RemovalListener removalListener; - private CounterMetric counter; - EhcacheEventListener(RemovalListener removalListener, CounterMetric counter) { + private EhcacheDiskCachingTier tier; + EhcacheEventListener(RemovalListener removalListener, EhcacheDiskCachingTier tier) { this.removalListener = removalListener; - this.counter = counter; // needed to handle count changes + this.tier = tier; // needed to handle count changes } @Override - public void onEvent(CacheEvent event) { - K key = event.getKey(); + public void onEvent(CacheEvent event) { + EhcacheKey ehcacheKey = event.getKey(); V oldValue = event.getOldValue(); V newValue = event.getNewValue(); EventType eventType = event.getType(); - System.out.println("I am eventing!!"); - // handle changing count for the disk tier if (oldValue == null && newValue != null) { - counter.inc(); + tier.countInc(); } else if (oldValue != null && newValue == null) { - counter.dec(); - } else { - int j; // breakpoint + tier.countDec(); } // handle creating a RemovalReason, unless eventType is CREATED @@ -63,6 +64,10 @@ public void onEvent(CacheEvent event) { default: reason = null; } - removalListener.onRemoval(new RemovalNotification(key, oldValue, reason)); + try { + K key = tier.convertEhcacheKeyToOriginal(ehcacheKey); + removalListener.onRemoval(new RemovalNotification(key, oldValue, reason)); + } catch (Exception ignored) {} + } } diff --git a/server/src/main/java/org/opensearch/indices/EhcacheKey.java b/server/src/main/java/org/opensearch/indices/EhcacheKey.java new file mode 100644 index 0000000000000..b0a8d660b98f6 --- /dev/null +++ b/server/src/main/java/org/opensearch/indices/EhcacheKey.java @@ -0,0 +1,54 @@ +/* + * 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.indices; + +import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.core.common.bytes.BytesReference; +import org.opensearch.core.common.io.stream.BytesStreamInput; +import org.opensearch.core.common.io.stream.Writeable; + +import java.io.IOException; +import java.io.Serializable; +import java.util.Arrays; + +public class EhcacheKey implements Serializable { + // the IndicesRequestCache.Key is not Serializable, but it is Writeable. + // We use the output stream's bytes in this wrapper class and implement the appropriate interfaces/methods. + // Unfortunately it's not possible to define this class as EhcacheKey and use that as ehcache keys, + // because of type erasure. However, the only context EhcacheKey objects would be compared to one another + // is when they are used for the same cache, so they will always refer to the same K. + private byte[] bytes; + + public EhcacheKey(Writeable key) throws IOException { + BytesStreamOutput os = new BytesStreamOutput(); // Should we pass in an expected size? If so, how big? + key.writeTo(os); + this.bytes = BytesReference.toBytes(os.bytes()); + } + + public byte[] getBytes() { + return this.bytes; + } + + @Override + public boolean equals(Object o) { + if (o == this) { + return true; + } + if (!(o instanceof EhcacheKey)) { + return false; + } + EhcacheKey other = (EhcacheKey) o; + return Arrays.equals(this.bytes, other.bytes); + } + + @Override + public int hashCode() { + return Arrays.hashCode(this.bytes); + } +} diff --git a/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java b/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java index 589c7dcfd9887..c240162423ee5 100644 --- a/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java +++ b/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java @@ -11,6 +11,7 @@ import org.opensearch.common.cache.RemovalListener; import org.opensearch.common.cache.RemovalNotification; import org.opensearch.common.cache.RemovalReason; +import org.opensearch.core.common.io.stream.Writeable; import java.util.Arrays; import java.util.List; @@ -22,10 +23,10 @@ * @param * @param */ -public class TieredCacheSpilloverStrategyHandler implements TieredCacheHandler, RemovalListener { +public class TieredCacheSpilloverStrategyHandler implements TieredCacheHandler, RemovalListener { private final OnHeapCachingTier onHeapCachingTier; - private final EhcacheDiskCachingTier diskCachingTier; // changed for testing + private final DiskCachingTier diskCachingTier; // changed for testing private final TieredCacheEventListener tieredCacheEventListener; /** @@ -109,7 +110,7 @@ public CachingTier getOnHeapCachingTier() { return this.onHeapCachingTier; } - public EhcacheDiskCachingTier getDiskCachingTier() { // change to CachingTier after debug + public DiskCachingTier getDiskCachingTier() { // change to CachingTier after debug return this.diskCachingTier; } @@ -147,7 +148,7 @@ public static class CacheValue { } } - public static class Builder { + public static class Builder { private OnHeapCachingTier onHeapCachingTier; private CachingTier diskCachingTier; private TieredCacheEventListener tieredCacheEventListener; From d21af15875ffc5ac34c0fa33577f47509ee9eabf Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Tue, 17 Oct 2023 10:00:15 -0700 Subject: [PATCH 16/22] final attempts at doing it generally before learning that wasnt even needed --- .../core/common/bytes/BytesReference.java | 2 +- .../org/opensearch/indices/CachingTier.java | 6 ++-- .../indices/EhcacheDiskCachingTier.java | 34 +++++++++++++++---- .../indices/EhcacheEventListener.java | 2 +- .../indices/IndicesRequestCache.java | 7 +++- .../indices/IndicesRequestCacheTests.java | 11 +++--- 6 files changed, 45 insertions(+), 17 deletions(-) diff --git a/libs/core/src/main/java/org/opensearch/core/common/bytes/BytesReference.java b/libs/core/src/main/java/org/opensearch/core/common/bytes/BytesReference.java index e07aca89339b9..f349baeef3cd7 100644 --- a/libs/core/src/main/java/org/opensearch/core/common/bytes/BytesReference.java +++ b/libs/core/src/main/java/org/opensearch/core/common/bytes/BytesReference.java @@ -55,7 +55,7 @@ * @opensearch.api */ @PublicApi(since = "1.0.0") -public interface BytesReference extends Comparable, ToXContentFragment { +public interface BytesReference extends Comparable, ToXContentFragment, Serializable { /** * Convert an {@link XContentBuilder} into a BytesReference. This method closes the builder, diff --git a/server/src/main/java/org/opensearch/indices/CachingTier.java b/server/src/main/java/org/opensearch/indices/CachingTier.java index 8c0dc0936f9dc..6726167fe469d 100644 --- a/server/src/main/java/org/opensearch/indices/CachingTier.java +++ b/server/src/main/java/org/opensearch/indices/CachingTier.java @@ -19,13 +19,13 @@ */ public interface CachingTier { - V get(K key) throws IOException; + V get(K key); - void put(K key, V value) throws IOException; + void put(K key, V value); V computeIfAbsent(K key, TieredCacheLoader loader) throws Exception; - void invalidate(K key) throws IOException; + void invalidate(K key); V compute(K key, TieredCacheLoader loader) throws Exception; diff --git a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java index 4a34fea847981..3fe9873e1ab80 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java @@ -31,6 +31,7 @@ import org.opensearch.common.metrics.CounterMetric; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.common.io.stream.BytesStreamInput; +import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.core.common.io.stream.Writeable; @@ -40,6 +41,7 @@ import java.util.Collections; public class EhcacheDiskCachingTier implements DiskCachingTier, RemovalListener { + // & Writeable.Reader ? private final PersistentCacheManager cacheManager; private final Cache cache; @@ -108,12 +110,16 @@ public EhcacheDiskCachingTier( } @Override - public V get(K key) throws IOException { + public V get(K key) { // I don't think we need to do the future stuff as the cache is threadsafe // if (keystore.contains(key.hashCode()) { long now = System.nanoTime(); - V value = cache.get(new EhcacheKey(key)); + V value = null; + try { + value = cache.get(new EhcacheKey(key)); + } catch (IOException ignored) { // do smth with this later + } double tookTimeMillis = ((double) (System.nanoTime() - now)) / 1000000; getTimeMillisEWMA.addValue(tookTimeMillis); return value; @@ -122,12 +128,15 @@ public V get(K key) throws IOException { } @Override - public void put(K key, V value) throws IOException { + public void put(K key, V value) { // No need to get old value, this is handled by EhcacheEventListener. // CheckDataResult policyResult = policy.checkData(value) // if (policyResult.isAccepted()) { - cache.put(new EhcacheKey(key), value); + try { + cache.put(new EhcacheKey(key), value); + } catch (IOException ignored) { // do smth with this later + } // keystore.add(key.hashCode()); // else { do something with policyResult.deniedReason()? } // } @@ -139,12 +148,15 @@ public V computeIfAbsent(K key, TieredCacheLoader loader) throws Exception } @Override - public void invalidate(K key) throws IOException { + public void invalidate(K key) { // keep keystore check to avoid unneeded disk seek // RemovalNotification is handled by EhcacheEventListener // if (keystore.contains(key.hashCode()) { - cache.remove(new EhcacheKey(key)); + try { + cache.remove(new EhcacheKey(key)); + } catch (IOException ignored) { // do smth with this later + } // keystore.remove(key.hashCode()); // } } @@ -207,6 +219,16 @@ public K convertEhcacheKeyToOriginal(EhcacheKey eKey) throws IOException { is.readBytes(bytes, 0, bytes.length); // we somehow have to use the Reader thing in the Writeable interface // otherwise its not generic + try { + K key = keyType.getDeclaredConstructor(new Class[]{StreamInput.class}).newInstance(); + // This cannot be the proper way to do it + // but if it is, make K extend some interface guaranteeing it has such a constructor (which im sure already exists somewhere in OS) + return key; + } catch (Exception e) { + System.out.println("Was unable to reconstruct EhcacheKey into K"); + e.printStackTrace(); + } + return null; } @Override diff --git a/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java b/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java index 6c355dba025e1..7ad8718222925 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java @@ -26,7 +26,7 @@ public class EhcacheEventListener implements CacheEventL // Receives key-value pairs (BytesReference, BytesReference), but must transform into (Key, BytesReference) // to send removal notifications private RemovalListener removalListener; - private EhcacheDiskCachingTier tier; + private EhcacheDiskCachingTier tier; EhcacheEventListener(RemovalListener removalListener, EhcacheDiskCachingTier tier) { this.removalListener = removalListener; this.tier = tier; // needed to handle count changes diff --git a/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java b/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java index 6caa83768a5eb..e55ce6ae73bb8 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java +++ b/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java @@ -294,7 +294,7 @@ interface CacheEntity extends Accountable, Writeable { * * @opensearch.internal */ - class Key implements Accountable, Writeable { + class Key implements Accountable, Writeable, Writeable.Reader { private final long BASE_RAM_BYTES_USED = RamUsageEstimator.shallowSizeOfInstance(Key.class); public final CacheEntity entity; // use as identity equality @@ -349,6 +349,11 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(readerCacheKeyUniqueId); out.writeBytesReference(value); } + + @Override + public Key read(StreamInput in) throws IOException { + return new Key(in); + } } private class CleanupKey implements IndexReader.ClosedListener { diff --git a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java index 87d8495a136d4..6ef23c7bb0790 100644 --- a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java +++ b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java @@ -181,10 +181,10 @@ public void testAddDirectToEhcache() throws Exception { cache.closeDiskTier(); } - public void testSimpleEhcache() throws Exception { + /*public void testSimpleEhcache() throws Exception { // for debug only, delete - CounterMetric count = new CounterMetric(); - String cacheAlias = "dummy"; + CounterMetric count = new CounterMetric(); + String cacheAlias = "dummy"; class DummyRemovalListener implements RemovalListener { public DummyRemovalListener() { } @@ -275,10 +275,11 @@ public void onRemoval(RemovalNotification notification) { IndicesRequestCache.Key key = new IndicesRequestCache.Key(entity, reader.getReaderCacheHelper().getKey(), termBytes);*/ - cacheManager.removeCache(cacheAlias); + /*cacheManager.removeCache(cacheAlias); cacheManager.close(); //IOUtils.close(reader, writer, dir); - } + + }*/ public void testSpillover() throws Exception { // fill the on-heap cache until we spill over From d152478ad7ad705b64be966b2e87f14009bfdcf0 Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Tue, 17 Oct 2023 11:29:52 -0700 Subject: [PATCH 17/22] Changed Ehcache disk tier implementation to not be generic, fixed persistent cache locking issue --- .../indices/EhcacheDiskCachingTier.java | 118 +++++++++++------- .../indices/EhcacheEventListener.java | 26 ++-- .../org/opensearch/indices/EhcacheKey.java | 3 - .../indices/IndicesRequestCache.java | 12 +- .../TieredCacheSpilloverStrategyHandler.java | 8 +- 5 files changed, 87 insertions(+), 80 deletions(-) diff --git a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java index 3fe9873e1ab80..5676a4278a45f 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java @@ -22,6 +22,7 @@ import org.ehcache.event.CacheEventListener; import org.ehcache.event.EventType; import org.ehcache.impl.config.executor.PooledExecutionServiceConfiguration; +import org.opensearch.action.admin.indices.exists.indices.IndicesExistsAction; import org.opensearch.common.ExponentiallyWeightedMovingAverage; import org.opensearch.common.cache.RemovalListener; import org.ehcache.Cache; @@ -40,17 +41,16 @@ import java.io.OutputStream; import java.util.Collections; -public class EhcacheDiskCachingTier implements DiskCachingTier, RemovalListener { +public class EhcacheDiskCachingTier implements DiskCachingTier, RemovalListener { // & Writeable.Reader ? - private final PersistentCacheManager cacheManager; - private final Cache cache; - private final Class keyType; // These are needed to pass to newCacheConfigurationBuilder + public static PersistentCacheManager cacheManager; + private Cache cache; + //private final Class keyType; // These are needed to pass to newCacheConfigurationBuilder //private final Class> ehcacheKeyType; - private final Class valueType; + //private final Class valueType; public final static String DISK_CACHE_FP = "disk_cache_tier"; // this should probably be defined somewhere else since we need to change security.policy based on its value - private RemovalListener removalListener; - private StatisticsService statsService; // non functional + private RemovalListener removalListener; private ExponentiallyWeightedMovingAverage getTimeMillisEWMA; private static final double GET_TIME_EWMA_ALPHA = 0.3; // This is the value used elsewhere in OpenSearch private static final int MIN_WRITE_THREADS = 0; @@ -58,7 +58,8 @@ public class EhcacheDiskCachingTier implements DiskCachi private static final String cacheAlias = "diskTier"; private final boolean isPersistent; private CounterMetric count; // number of entries in cache - private final EhcacheEventListener listener; + private final EhcacheEventListener listener; + private final IndicesRequestCache indicesRequestCache; // only used to create new Keys // private RBMIntKeyLookupStore keystore; // private CacheTierPolicy[] policies; // private IndicesRequestCacheDiskTierPolicy policy; @@ -67,18 +68,44 @@ public EhcacheDiskCachingTier( boolean isPersistent, long maxWeightInBytes, long maxKeystoreWeightInBytes, - Class keyType, + IndicesRequestCache indicesRequestCache + //Class keyType, //Class> ehcacheKeyType, - Class valueType + //Class valueType ) { this.isPersistent = isPersistent; - this.keyType = keyType; + //this.keyType = keyType; //this.ehcacheKeyType = ehcacheKeyType; - this.valueType = valueType; + //this.valueType = valueType; this.count = new CounterMetric(); - this.listener = new EhcacheEventListener(this, this); - statsService = new DefaultStatisticsService(); + this.listener = new EhcacheEventListener(this, this); + this.indicesRequestCache = indicesRequestCache; + getManager(); + getOrCreateCache(isPersistent, maxWeightInBytes); + this.getTimeMillisEWMA = new ExponentiallyWeightedMovingAverage(GET_TIME_EWMA_ALPHA, 10); + + // this.keystore = new RBMIntKeyLookupStore((int) Math.pow(2, 28), maxKeystoreWeightInBytes); + // this.policies = new CacheTierPolicy[]{ new IndicesRequestCacheTookTimePolicy(settings, clusterSettings) }; + // this.policy = new IndicesRequestCacheDiskTierPolicy(this.policies, true); + } + + public static void getManager() { + // based on https://stackoverflow.com/questions/53756412/ehcache-org-ehcache-statetransitionexception-persistence-directory-already-lo + // resolving double-initialization issue when using OpenSearchSingleNodeTestCase + if (cacheManager == null) { + PooledExecutionServiceConfiguration threadConfig = PooledExecutionServiceConfigurationBuilder.newPooledExecutionServiceConfigurationBuilder() + .defaultPool("default", MIN_WRITE_THREADS, MAX_WRITE_THREADS) + .build(); + + cacheManager = CacheManagerBuilder.newCacheManagerBuilder() + .using(threadConfig) + .with(CacheManagerBuilder.persistence(DISK_CACHE_FP) + ).build(true); + } + } + + private void getOrCreateCache(boolean isPersistent, long maxWeightInBytes) { // our EhcacheEventListener should receive events every time an entry is changed CacheEventListenerConfigurationBuilder listenerConfig = CacheEventListenerConfigurationBuilder .newEventListenerConfiguration(listener, @@ -87,35 +114,27 @@ public EhcacheDiskCachingTier( EventType.REMOVED, EventType.UPDATED, EventType.CREATED) - .ordered().asynchronous(); // ordered() has some performance penalty as compared to unordered(), we can also use synchronous() - - PooledExecutionServiceConfiguration threadConfig = PooledExecutionServiceConfigurationBuilder.newPooledExecutionServiceConfigurationBuilder() - .defaultPool("default", MIN_WRITE_THREADS, MAX_WRITE_THREADS) - .build(); - - this.cacheManager = CacheManagerBuilder.newCacheManagerBuilder() - .using(statsService) // https://stackoverflow.com/questions/40453859/how-to-get-ehcache-3-1-statistics - .using(threadConfig) - .with(CacheManagerBuilder.persistence(DISK_CACHE_FP)) - .withCache(cacheAlias, CacheConfigurationBuilder.newCacheConfigurationBuilder( - EhcacheKey.class, valueType, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(maxWeightInBytes, MemoryUnit.B, isPersistent)) - .withService(listenerConfig) - ).build(true); - this.cache = cacheManager.getCache(cacheAlias, EhcacheKey.class, valueType); - this.getTimeMillisEWMA = new ExponentiallyWeightedMovingAverage(GET_TIME_EWMA_ALPHA, 10); + .ordered().asynchronous(); + // ordered() has some performance penalty as compared to unordered(), we can also use synchronous() - // this.keystore = new RBMIntKeyLookupStore((int) Math.pow(2, 28), maxKeystoreWeightInBytes); - // this.policies = new CacheTierPolicy[]{ new IndicesRequestCacheTookTimePolicy(settings, clusterSettings) }; - // this.policy = new IndicesRequestCacheDiskTierPolicy(this.policies, true); + try { + cache = cacheManager.createCache(cacheAlias, + CacheConfigurationBuilder.newCacheConfigurationBuilder( + EhcacheKey.class, BytesReference.class, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(maxWeightInBytes, MemoryUnit.B, isPersistent)) + .withService(listenerConfig)); + } catch (IllegalArgumentException e) { + // Thrown when the cache already exists, which may happen in test cases + cache = cacheManager.getCache(cacheAlias, EhcacheKey.class, BytesReference.class); + } } @Override - public V get(K key) { + public BytesReference get(IndicesRequestCache.Key key) { // I don't think we need to do the future stuff as the cache is threadsafe // if (keystore.contains(key.hashCode()) { long now = System.nanoTime(); - V value = null; + BytesReference value = null; try { value = cache.get(new EhcacheKey(key)); } catch (IOException ignored) { // do smth with this later @@ -128,7 +147,7 @@ public V get(K key) { } @Override - public void put(K key, V value) { + public void put(IndicesRequestCache.Key key, BytesReference value) { // No need to get old value, this is handled by EhcacheEventListener. // CheckDataResult policyResult = policy.checkData(value) @@ -143,12 +162,12 @@ public void put(K key, V value) { } @Override - public V computeIfAbsent(K key, TieredCacheLoader loader) throws Exception { + public BytesReference computeIfAbsent(IndicesRequestCache.Key key, TieredCacheLoader loader) throws Exception { return null; // should not need to fill out, Cache.computeIfAbsent is always used } @Override - public void invalidate(K key) { + public void invalidate(IndicesRequestCache.Key key) { // keep keystore check to avoid unneeded disk seek // RemovalNotification is handled by EhcacheEventListener @@ -162,12 +181,12 @@ public void invalidate(K key) { } @Override - public V compute(K key, TieredCacheLoader loader) throws Exception { + public BytesReference compute(IndicesRequestCache.Key key, TieredCacheLoader loader) throws Exception { return null; // should not need to fill out, Cache.compute is always used } @Override - public void setRemovalListener(RemovalListener removalListener) { + public void setRemovalListener(RemovalListener removalListener) { this.removalListener = removalListener; // this is passed the spillover strategy, same as on-heap } @@ -177,7 +196,7 @@ public void invalidateAll() { } @Override - public Iterable keys() { + public Iterable keys() { // ehcache doesn't provide a method like this, because it might be a huge number of keys that consume all // the memory. Do we want this method for disk tier? return Collections::emptyIterator; @@ -201,7 +220,7 @@ public TierType getTierType() { } @Override - public void onRemoval(RemovalNotification notification) { + public void onRemoval(RemovalNotification notification) { removalListener.onRemoval(notification); } @@ -213,19 +232,16 @@ public boolean isPersistent() { return isPersistent; } - public K convertEhcacheKeyToOriginal(EhcacheKey eKey) throws IOException { + public IndicesRequestCache.Key convertEhcacheKeyToOriginal(EhcacheKey eKey) throws IOException { BytesStreamInput is = new BytesStreamInput(); byte[] bytes = eKey.getBytes(); is.readBytes(bytes, 0, bytes.length); // we somehow have to use the Reader thing in the Writeable interface // otherwise its not generic try { - K key = keyType.getDeclaredConstructor(new Class[]{StreamInput.class}).newInstance(); - // This cannot be the proper way to do it - // but if it is, make K extend some interface guaranteeing it has such a constructor (which im sure already exists somewhere in OS) - return key; + return indicesRequestCache.new Key(is); } catch (Exception e) { - System.out.println("Was unable to reconstruct EhcacheKey into K"); + System.out.println("Was unable to reconstruct EhcacheKey into Key"); e.printStackTrace(); } return null; @@ -238,4 +254,10 @@ public void close() { cacheManager.removeCache(cacheAlias); cacheManager.close(); } + + public void destroy() throws Exception { + // Close the cache and delete any persistent data associated with it + // Might also be appropriate after standalone tests + cacheManager.destroy(); + } } diff --git a/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java b/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java index 7ad8718222925..567daa4a266ce 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java @@ -14,28 +14,22 @@ import org.opensearch.common.cache.RemovalListener; import org.opensearch.common.cache.RemovalNotification; import org.opensearch.common.cache.RemovalReason; -import org.opensearch.common.metrics.CounterMetric; -import org.opensearch.core.common.io.stream.BytesStreamInput; -import org.opensearch.core.common.io.stream.StreamInput; -import org.opensearch.core.common.io.stream.Writeable; -import org.opensearch.indices.EhcacheDiskCachingTier; +import org.opensearch.core.common.bytes.BytesReference; -// moved to another file for testing flexibility purposes - -public class EhcacheEventListener implements CacheEventListener { +public class EhcacheEventListener implements CacheEventListener { // Receives key-value pairs (BytesReference, BytesReference), but must transform into (Key, BytesReference) // to send removal notifications - private RemovalListener removalListener; - private EhcacheDiskCachingTier tier; - EhcacheEventListener(RemovalListener removalListener, EhcacheDiskCachingTier tier) { + private final RemovalListener removalListener; + private final EhcacheDiskCachingTier tier; + EhcacheEventListener(RemovalListener removalListener, EhcacheDiskCachingTier tier) { this.removalListener = removalListener; this.tier = tier; // needed to handle count changes } @Override - public void onEvent(CacheEvent event) { + public void onEvent(CacheEvent event) { EhcacheKey ehcacheKey = event.getKey(); - V oldValue = event.getOldValue(); - V newValue = event.getNewValue(); + BytesReference oldValue = event.getOldValue(); + BytesReference newValue = event.getNewValue(); EventType eventType = event.getType(); // handle changing count for the disk tier @@ -65,8 +59,8 @@ public void onEvent(CacheEvent event) { reason = null; } try { - K key = tier.convertEhcacheKeyToOriginal(ehcacheKey); - removalListener.onRemoval(new RemovalNotification(key, oldValue, reason)); + IndicesRequestCache.Key key = tier.convertEhcacheKeyToOriginal(ehcacheKey); + removalListener.onRemoval(new RemovalNotification<>(key, oldValue, reason)); } catch (Exception ignored) {} } diff --git a/server/src/main/java/org/opensearch/indices/EhcacheKey.java b/server/src/main/java/org/opensearch/indices/EhcacheKey.java index b0a8d660b98f6..43695f4bcc26f 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheKey.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheKey.java @@ -20,9 +20,6 @@ public class EhcacheKey implements Serializable { // the IndicesRequestCache.Key is not Serializable, but it is Writeable. // We use the output stream's bytes in this wrapper class and implement the appropriate interfaces/methods. - // Unfortunately it's not possible to define this class as EhcacheKey and use that as ehcache keys, - // because of type erasure. However, the only context EhcacheKey objects would be compared to one another - // is when they are used for the same cache, so they will always refer to the same K. private byte[] bytes; public EhcacheKey(Writeable key) throws IOException { diff --git a/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java b/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java index e55ce6ae73bb8..a38e9d71fb982 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java +++ b/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java @@ -54,7 +54,6 @@ import java.io.Closeable; import java.io.IOException; -import java.io.Serializable; import java.util.Collection; import java.util.Collections; import java.util.HashSet; @@ -132,8 +131,8 @@ public final class IndicesRequestCache implements TieredCacheEventListener diskCachingTier; - diskCachingTier = new EhcacheDiskCachingTier<>(true, diskTierWeight, 0, Key.class, BytesReference.class); + EhcacheDiskCachingTier diskCachingTier; + diskCachingTier = new EhcacheDiskCachingTier(false, diskTierWeight, 0, this); // making non-persistent for now tieredCacheHandler = new TieredCacheSpilloverStrategyHandler.Builder().setOnHeapCachingTier( openSearchOnHeapCache ).setOnDiskCachingTier(diskCachingTier).setTieredCacheEventListener(this).build(); @@ -294,7 +293,7 @@ interface CacheEntity extends Accountable, Writeable { * * @opensearch.internal */ - class Key implements Accountable, Writeable, Writeable.Reader { + class Key implements Accountable, Writeable { private final long BASE_RAM_BYTES_USED = RamUsageEstimator.shallowSizeOfInstance(Key.class); public final CacheEntity entity; // use as identity equality @@ -349,11 +348,6 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(readerCacheKeyUniqueId); out.writeBytesReference(value); } - - @Override - public Key read(StreamInput in) throws IOException { - return new Key(in); - } } private class CleanupKey implements IndexReader.ClosedListener { diff --git a/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java b/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java index c240162423ee5..203f72c8ff979 100644 --- a/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java +++ b/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java @@ -36,7 +36,7 @@ public class TieredCacheSpilloverStrategyHandler impleme private TieredCacheSpilloverStrategyHandler( OnHeapCachingTier onHeapCachingTier, - EhcacheDiskCachingTier diskCachingTier, + DiskCachingTier diskCachingTier, // changed to EhcacheDiskCachingTier from CachingTier, to enable close() method, which is needed for tests. Implement close() in CachingTier or DiskCachingTier? TieredCacheEventListener tieredCacheEventListener ) { @@ -150,7 +150,7 @@ public static class CacheValue { public static class Builder { private OnHeapCachingTier onHeapCachingTier; - private CachingTier diskCachingTier; + private DiskCachingTier diskCachingTier; private TieredCacheEventListener tieredCacheEventListener; public Builder() {} @@ -160,7 +160,7 @@ public Builder setOnHeapCachingTier(OnHeapCachingTier onHeapCachingT return this; } - public Builder setOnDiskCachingTier(CachingTier diskCachingTier) { + public Builder setOnDiskCachingTier(DiskCachingTier diskCachingTier) { this.diskCachingTier = diskCachingTier; return this; } @@ -173,7 +173,7 @@ public Builder setTieredCacheEventListener(TieredCacheEventListener public TieredCacheSpilloverStrategyHandler build() { return new TieredCacheSpilloverStrategyHandler( this.onHeapCachingTier, - (EhcacheDiskCachingTier) this.diskCachingTier, // not sure why it was yelling about this, it already is an EhcacheDiskCachingTier + this.diskCachingTier, // not sure why it was yelling about this, it already is an EhcacheDiskCachingTier this.tieredCacheEventListener ); } From 2d200fcedd8326b45276a8e0a9f5f0eaeea48dba Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Tue, 17 Oct 2023 12:33:53 -0700 Subject: [PATCH 18/22] Fixed multiple disk tier instance issue, made spillover test pass --- .../common/metrics/CounterMetric.java | 1 - .../indices/EhcacheDiskCachingTier.java | 34 +++-- .../indices/EhcacheEventListener.java | 2 +- .../org/opensearch/indices/EhcacheKey.java | 1 - .../TieredCacheSpilloverStrategyHandler.java | 11 +- .../indices/IndicesRequestCacheTests.java | 139 +++--------------- 6 files changed, 51 insertions(+), 137 deletions(-) diff --git a/server/src/main/java/org/opensearch/common/metrics/CounterMetric.java b/server/src/main/java/org/opensearch/common/metrics/CounterMetric.java index 33fc5e32e9c60..2e5eae5ceebe0 100644 --- a/server/src/main/java/org/opensearch/common/metrics/CounterMetric.java +++ b/server/src/main/java/org/opensearch/common/metrics/CounterMetric.java @@ -63,5 +63,4 @@ public void dec(long n) { public long count() { return counter.sum(); } - } diff --git a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java index 5676a4278a45f..190cfa1b9f7cc 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java @@ -9,40 +9,30 @@ package org.opensearch.indices; import org.ehcache.PersistentCacheManager; +import org.ehcache.config.CacheRuntimeConfiguration; import org.ehcache.config.builders.CacheConfigurationBuilder; import org.ehcache.config.builders.CacheEventListenerConfigurationBuilder; import org.ehcache.config.builders.CacheManagerBuilder; import org.ehcache.config.builders.PooledExecutionServiceConfigurationBuilder; import org.ehcache.config.builders.ResourcePoolsBuilder; import org.ehcache.config.units.MemoryUnit; -import org.ehcache.core.internal.statistics.DefaultStatisticsService; -import org.ehcache.core.spi.service.StatisticsService; -import org.ehcache.core.statistics.TierStatistics; -import org.ehcache.event.CacheEvent; -import org.ehcache.event.CacheEventListener; +import org.ehcache.event.EventFiring; +import org.ehcache.event.EventOrdering; import org.ehcache.event.EventType; import org.ehcache.impl.config.executor.PooledExecutionServiceConfiguration; -import org.opensearch.action.admin.indices.exists.indices.IndicesExistsAction; import org.opensearch.common.ExponentiallyWeightedMovingAverage; import org.opensearch.common.cache.RemovalListener; import org.ehcache.Cache; import org.opensearch.common.cache.RemovalNotification; -import org.opensearch.common.cache.RemovalReason; -import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.metrics.CounterMetric; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.common.io.stream.BytesStreamInput; -import org.opensearch.core.common.io.stream.StreamInput; -import org.opensearch.core.common.io.stream.StreamOutput; -import org.opensearch.core.common.io.stream.Writeable; import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; import java.util.Collections; +import java.util.EnumSet; public class EhcacheDiskCachingTier implements DiskCachingTier, RemovalListener { - // & Writeable.Reader ? public static PersistentCacheManager cacheManager; private Cache cache; @@ -124,7 +114,22 @@ private void getOrCreateCache(boolean isPersistent, long maxWeightInBytes) { .withService(listenerConfig)); } catch (IllegalArgumentException e) { // Thrown when the cache already exists, which may happen in test cases + // In this case the listener is configured to send messages to some other disk tier instance, which we don't want + // (it was set up unnecessarily by the test case) + + // change config of existing cache to use this listener rather than the one instantiated by the test case cache = cacheManager.getCache(cacheAlias, EhcacheKey.class, BytesReference.class); + // cache.getRuntimeConfiguration().cacheConfigurationListenerList contains the old listener, but it's private + // and theres no method to clear it unless you have the actual listener object, so it has to stay i think + + cache.getRuntimeConfiguration().registerCacheEventListener(listener, EventOrdering.ORDERED, EventFiring.ASYNCHRONOUS, + EnumSet.of( + EventType.EVICTED, + EventType.EXPIRED, + EventType.REMOVED, + EventType.UPDATED, + EventType.CREATED)); + int k = 1; } } @@ -204,6 +209,7 @@ public Iterable keys() { @Override public int count() { + int j = 0; return (int) count.count(); } diff --git a/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java b/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java index 567daa4a266ce..e14102fd80ed5 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheEventListener.java @@ -17,7 +17,7 @@ import org.opensearch.core.common.bytes.BytesReference; public class EhcacheEventListener implements CacheEventListener { - // Receives key-value pairs (BytesReference, BytesReference), but must transform into (Key, BytesReference) + // Receives key-value pairs (EhcacheKey, BytesReference), but must transform into (Key, BytesReference) // to send removal notifications private final RemovalListener removalListener; private final EhcacheDiskCachingTier tier; diff --git a/server/src/main/java/org/opensearch/indices/EhcacheKey.java b/server/src/main/java/org/opensearch/indices/EhcacheKey.java index 43695f4bcc26f..f8fa87932d66f 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheKey.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheKey.java @@ -10,7 +10,6 @@ import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.core.common.bytes.BytesReference; -import org.opensearch.core.common.io.stream.BytesStreamInput; import org.opensearch.core.common.io.stream.Writeable; import java.io.IOException; diff --git a/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java b/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java index 203f72c8ff979..6a4aa812cf010 100644 --- a/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java +++ b/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java @@ -91,6 +91,15 @@ public long count() { return totalCount; } + public long count(TierType tierType) { + for (CachingTier cachingTier : cachingTierList) { + if (cachingTier.getTierType() == tierType) { + return cachingTier.count(); + } + } + return -1L; + } + @Override public void onRemoval(RemovalNotification notification) { if (RemovalReason.EVICTED.equals(notification.getRemovalReason())) { @@ -173,7 +182,7 @@ public Builder setTieredCacheEventListener(TieredCacheEventListener public TieredCacheSpilloverStrategyHandler build() { return new TieredCacheSpilloverStrategyHandler( this.onHeapCachingTier, - this.diskCachingTier, // not sure why it was yelling about this, it already is an EhcacheDiskCachingTier + this.diskCachingTier, this.tieredCacheEventListener ); } diff --git a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java index 6ef23c7bb0790..35da7bac938e6 100644 --- a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java +++ b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java @@ -59,11 +59,8 @@ import org.ehcache.event.EventType; import org.ehcache.impl.config.executor.PooledExecutionServiceConfiguration; import org.opensearch.common.CheckedSupplier; -import org.opensearch.common.cache.RemovalListener; -import org.opensearch.common.cache.RemovalNotification; import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.lucene.index.OpenSearchDirectoryReader; -import org.opensearch.common.metrics.CounterMetric; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.io.IOUtils; import org.opensearch.core.common.bytes.AbstractBytesReference; @@ -164,128 +161,29 @@ public void testAddDirectToEhcache() throws Exception { DirectoryReader reader = OpenSearchDirectoryReader.wrap(DirectoryReader.open(writer), new ShardId("foo", "bar", 1)); AtomicBoolean indexShard = new AtomicBoolean(true); TestEntity entity = new TestEntity(requestCacheStats, indexShard); - Loader loader = new Loader(reader, 0); - IndicesRequestCache.Key[] keys = new IndicesRequestCache.Key[9]; TermQueryBuilder termQuery = new TermQueryBuilder("id", "0"); BytesReference termBytes = XContentHelper.toXContent(termQuery, MediaTypeRegistry.JSON, false); String rKey = ((OpenSearchDirectoryReader) reader).getDelegatingCacheHelper().getDelegatingCacheKey().getId().toString(); IndicesRequestCache.Key key = cache.new Key(entity, termBytes, rKey); - //TestBytesReference value = new TestBytesReference(124); BytesReference value = new BytesArray(new byte[]{0}); cache.tieredCacheHandler.getDiskCachingTier().put(key, value); - System.out.println("Size: " + cache.tieredCacheHandler.getDiskCachingTier().count()); + BytesReference res = cache.tieredCacheHandler.getDiskCachingTier().get(key); + assertEquals(value, res); + assertEquals(1, cache.tieredCacheHandler.count(TierType.DISK)); IOUtils.close(reader, writer, dir, cache); cache.closeDiskTier(); } - /*public void testSimpleEhcache() throws Exception { - // for debug only, delete - CounterMetric count = new CounterMetric(); - String cacheAlias = "dummy"; - - class DummyRemovalListener implements RemovalListener { - public DummyRemovalListener() { } - @Override - public void onRemoval(RemovalNotification notification) { - System.out.println(":)"); - } - } - - CacheEventListenerConfigurationBuilder listenerConfig = CacheEventListenerConfigurationBuilder - .newEventListenerConfiguration(new EhcacheEventListener(new DummyRemovalListener(), count), - EventType.EVICTED, - EventType.EXPIRED, - EventType.REMOVED, - EventType.UPDATED, - EventType.CREATED) - .ordered().asynchronous(); // ordered() has some performance penalty as compared to unordered(), we can also use synchronous() - - StatisticsService statsService = new DefaultStatisticsService(); - - PooledExecutionServiceConfiguration threadConfig = PooledExecutionServiceConfigurationBuilder.newPooledExecutionServiceConfigurationBuilder() - .defaultPool("default", 0, 4) - .build(); - - PersistentCacheManager cacheManager; - - boolean doIntCache = false; - - if (doIntCache) { - cacheManager = CacheManagerBuilder.newCacheManagerBuilder() - .using(statsService) // https://stackoverflow.com/questions/40453859/how-to-get-ehcache-3-1-statistics - .using(threadConfig) - .with(CacheManagerBuilder.persistence(EhcacheDiskCachingTier.DISK_CACHE_FP)) - .withCache(cacheAlias, CacheConfigurationBuilder.newCacheConfigurationBuilder( - Integer.class, String.class, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(10, MemoryUnit.MB, false)) - .withService(listenerConfig) - ).build(true); - Cache integerCache = cacheManager.getCache(cacheAlias, Integer.class, String.class); - - integerCache.put(0, "blorp"); - System.out.println("Counter value = " + count.count()); - String res = integerCache.get(0); - System.out.println("Got result " + res); - System.out.println("Counter value = " + count.count()); - } else { - cacheManager = CacheManagerBuilder.newCacheManagerBuilder() - .using(statsService) // https://stackoverflow.com/questions/40453859/how-to-get-ehcache-3-1-statistics - .using(threadConfig) - .with(CacheManagerBuilder.persistence(EhcacheDiskCachingTier.DISK_CACHE_FP)) - .withCache(cacheAlias, CacheConfigurationBuilder.newCacheConfigurationBuilder( - DummySerializableKey.class, String.class, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(10, MemoryUnit.MB, false)) - .withService(listenerConfig) - ).build(true); - Cache cache = cacheManager.getCache(cacheAlias, DummySerializableKey.class, String.class); - - DummySerializableKey key = new DummySerializableKey(Integer.valueOf(0), "blah"); - cache.put(key, "blorp"); - System.out.println("Counter value = " + count.count()); - String res = cache.get(key); - System.out.println("Got result " + res); - System.out.println("Counter value = " + count.count()); - TierStatistics ts = statsService.getCacheStatistics(cacheAlias).getTierStatistics().get("Disk"); - System.out.println("self-reported count = " + ts.getMappings()); - System.out.println("self-reported misses = " + ts.getMisses()); - System.out.println("self-reported hits = " + ts.getHits()); - - List> foos = new ArrayList<>(); - for(Cache.Entry entry : cache) { - foos.add(entry); - } - int j = 0; - j++; - System.out.println(j); - } - - /*Directory dir = newDirectory(); - IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig()); - writer.addDocument(newDoc(0, "foo")); - DirectoryReader reader = OpenSearchDirectoryReader.wrap(DirectoryReader.open(writer), new ShardId("foo", "bar", 1)); - AtomicBoolean indexShard = new AtomicBoolean(true); - ShardRequestCache requestCacheStats = new ShardRequestCache(); - TestEntity entity = new TestEntity(requestCacheStats, indexShard); - Loader loader = new Loader(reader, 0); - System.out.println("On-heap cache size at start = " + requestCacheStats.stats().getMemorySizeInBytes()); - IndicesRequestCache.Key[] keys = new IndicesRequestCache.Key[9]; - TermQueryBuilder termQuery = new TermQueryBuilder("id", "0"); - BytesReference termBytes = XContentHelper.toXContent(termQuery, MediaTypeRegistry.JSON, false); - IndicesRequestCache.Key key = new IndicesRequestCache.Key(entity, reader.getReaderCacheHelper().getKey(), termBytes);*/ - - - /*cacheManager.removeCache(cacheAlias); - cacheManager.close(); - //IOUtils.close(reader, writer, dir); - - }*/ - public void testSpillover() throws Exception { // fill the on-heap cache until we spill over ShardRequestCache requestCacheStats = new ShardRequestCache(); Settings.Builder settingsBuilder = Settings.builder(); - long heapSizeBytes = 1000; // each of these queries is 115 bytes, so we can fit 8 in the heap cache + long heapSizeBytes = 1000; // each of these queries is 131 bytes, so we can fit 7 in the heap cache + int heapKeySize = 131; + int maxNumInHeap = 1000 / heapKeySize; settingsBuilder.put("indices.requests.cache.size", new ByteSizeValue(heapSizeBytes)); IndicesRequestCache cache = new IndicesRequestCache(settingsBuilder.build(), getInstanceFromNode(IndicesService.class)); @@ -298,8 +196,8 @@ public void testSpillover() throws Exception { TestEntity entity = new TestEntity(requestCacheStats, indexShard); Loader loader = new Loader(reader, 0); System.out.println("On-heap cache size at start = " + requestCacheStats.stats().getMemorySizeInBytes()); - IndicesRequestCache.Key[] keys = new IndicesRequestCache.Key[9]; - for (int i = 0; i < 9; i++) { + IndicesRequestCache.Key[] keys = new IndicesRequestCache.Key[maxNumInHeap + 1]; + for (int i = 0; i < maxNumInHeap + 1; i++) { TermQueryBuilder termQuery = new TermQueryBuilder("id", String.valueOf(i)); BytesReference termBytes = XContentHelper.toXContent(termQuery, MediaTypeRegistry.JSON, false); String rKey = ((OpenSearchDirectoryReader) reader).getDelegatingCacheHelper().getDelegatingCacheKey().getId().toString(); @@ -310,15 +208,18 @@ public void testSpillover() throws Exception { } // attempt to get value from disk cache, the first key should have been evicted BytesReference firstValue = cache.tieredCacheHandler.get(keys[0]); - System.out.println("Final on-heap cache size = " + requestCacheStats.stats().getMemorySizeInBytes()); // is correctly 920 - //System.out.println("Final self-reported disk size = " + cache.tieredCacheHandler.getDiskWeightBytes()); // is 0, should be 115 - System.out.println("On-heap tier evictions = " + requestCacheStats.stats().getEvictions()); // is correctly 1 - System.out.println("Disk tier hits = " + requestCacheStats.stats(TierType.DISK).getHitCount()); // should be 1, is 0 bc keys not serializable - System.out.println("Disk tier misses = " + requestCacheStats.stats(TierType.DISK).getMissCount()); // should be 9, is 10 bc keys not serializable - //System.out.println("Disk tier self-reported misses = " + cache.tieredCacheHandler.getDiskCachingTier().getMisses()); // should be same as other one - System.out.println("On-heap tier hits = " + requestCacheStats.stats().getHitCount()); // is correctly 0 - System.out.println("On-heap tier misses = " + requestCacheStats.stats().getMissCount()); // is correctly 10 - System.out.println("Disk count = " + cache.tieredCacheHandler.getDiskCachingTier().count()); // should be 1, is 0 + + assertEquals(maxNumInHeap * heapKeySize, requestCacheStats.stats().getMemorySizeInBytes()); + // TODO: disk weight bytes + assertEquals(1, requestCacheStats.stats().getEvictions()); + assertEquals(1, requestCacheStats.stats(TierType.DISK).getHitCount()); + assertEquals(maxNumInHeap + 1, requestCacheStats.stats(TierType.DISK).getMissCount()); + assertEquals(0, requestCacheStats.stats().getHitCount()); + assertEquals(maxNumInHeap + 2, requestCacheStats.stats().getMissCount()); + assertEquals(maxNumInHeap, cache.tieredCacheHandler.count(TierType.ON_HEAP)); + assertEquals(1, cache.tieredCacheHandler.count(TierType.DISK)); + + // more? IOUtils.close(reader, writer, dir, cache); cache.closeDiskTier(); } From 8f65db48e689b2c3f8b7834a127ac2ad3c6f8064 Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Tue, 17 Oct 2023 15:11:48 -0700 Subject: [PATCH 19/22] Removed persistence, cleaned up --- .../opensearch/indices/DiskCachingTier.java | 6 ++ .../indices/EhcacheDiskCachingTier.java | 97 ++++++++----------- .../indices/EhcacheEventListener.java | 1 - .../indices/IndicesRequestCache.java | 4 +- .../indices/TieredCacheHandler.java | 2 + .../TieredCacheSpilloverStrategyHandler.java | 5 + .../indices/IndicesRequestCacheTests.java | 68 ++++++++----- 7 files changed, 99 insertions(+), 84 deletions(-) diff --git a/server/src/main/java/org/opensearch/indices/DiskCachingTier.java b/server/src/main/java/org/opensearch/indices/DiskCachingTier.java index de88c44cc1e8f..b1b5b03ed13cd 100644 --- a/server/src/main/java/org/opensearch/indices/DiskCachingTier.java +++ b/server/src/main/java/org/opensearch/indices/DiskCachingTier.java @@ -13,4 +13,10 @@ public interface DiskCachingTier extends CachingTier { * Closes the disk tier. */ void close(); + + /** + * Get the EWMA time in milliseconds for a get(). + * @return + */ + double getTimeMillisEWMA(); } diff --git a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java index 190cfa1b9f7cc..ce0fc52c9a96e 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java @@ -9,7 +9,6 @@ package org.opensearch.indices; import org.ehcache.PersistentCacheManager; -import org.ehcache.config.CacheRuntimeConfiguration; import org.ehcache.config.builders.CacheConfigurationBuilder; import org.ehcache.config.builders.CacheEventListenerConfigurationBuilder; import org.ehcache.config.builders.CacheManagerBuilder; @@ -36,17 +35,13 @@ public class EhcacheDiskCachingTier implements DiskCachingTier cache; - //private final Class keyType; // These are needed to pass to newCacheConfigurationBuilder - //private final Class> ehcacheKeyType; - //private final Class valueType; - public final static String DISK_CACHE_FP = "disk_cache_tier"; // this should probably be defined somewhere else since we need to change security.policy based on its value + public final static String DISK_CACHE_FP = "disk_cache_tier"; // Placeholder. this should probably be defined somewhere else, since we need to change security.policy based on its value private RemovalListener removalListener; private ExponentiallyWeightedMovingAverage getTimeMillisEWMA; private static final double GET_TIME_EWMA_ALPHA = 0.3; // This is the value used elsewhere in OpenSearch private static final int MIN_WRITE_THREADS = 0; private static final int MAX_WRITE_THREADS = 4; // Max number of threads for the PooledExecutionService which handles writes private static final String cacheAlias = "diskTier"; - private final boolean isPersistent; private CounterMetric count; // number of entries in cache private final EhcacheEventListener listener; private final IndicesRequestCache indicesRequestCache; // only used to create new Keys @@ -55,24 +50,23 @@ public class EhcacheDiskCachingTier implements DiskCachingTier keyType, - //Class> ehcacheKeyType, - //Class valueType ) { - this.isPersistent = isPersistent; - //this.keyType = keyType; - //this.ehcacheKeyType = ehcacheKeyType; - //this.valueType = valueType; this.count = new CounterMetric(); this.listener = new EhcacheEventListener(this, this); this.indicesRequestCache = indicesRequestCache; getManager(); - getOrCreateCache(isPersistent, maxWeightInBytes); + try { + cacheManager.destroyCache(cacheAlias); + } catch (Exception e) { + System.out.println("Unable to destroy cache!!"); + e.printStackTrace(); + // do actual logging later + } + createCache(maxWeightInBytes); this.getTimeMillisEWMA = new ExponentiallyWeightedMovingAverage(GET_TIME_EWMA_ALPHA, 10); // this.keystore = new RBMIntKeyLookupStore((int) Math.pow(2, 28), maxKeystoreWeightInBytes); @@ -83,19 +77,31 @@ public EhcacheDiskCachingTier( public static void getManager() { // based on https://stackoverflow.com/questions/53756412/ehcache-org-ehcache-statetransitionexception-persistence-directory-already-lo // resolving double-initialization issue when using OpenSearchSingleNodeTestCase - if (cacheManager == null) { - PooledExecutionServiceConfiguration threadConfig = PooledExecutionServiceConfigurationBuilder.newPooledExecutionServiceConfigurationBuilder() - .defaultPool("default", MIN_WRITE_THREADS, MAX_WRITE_THREADS) - .build(); - - cacheManager = CacheManagerBuilder.newCacheManagerBuilder() - .using(threadConfig) - .with(CacheManagerBuilder.persistence(DISK_CACHE_FP) - ).build(true); + if (cacheManager != null) { + try { + try { + cacheManager.close(); + } catch (IllegalStateException e) { + System.out.println("Cache was uninitialized, skipping close() and moving to destroy()"); + } + cacheManager.destroy(); + } catch (Exception e) { + System.out.println("Was unable to destroy cache manager"); + e.printStackTrace(); + // actual logging later + } } + PooledExecutionServiceConfiguration threadConfig = PooledExecutionServiceConfigurationBuilder.newPooledExecutionServiceConfigurationBuilder() + .defaultPool("default", MIN_WRITE_THREADS, MAX_WRITE_THREADS) + .build(); + + cacheManager = CacheManagerBuilder.newCacheManagerBuilder() + .using(threadConfig) + .with(CacheManagerBuilder.persistence(DISK_CACHE_FP) + ).build(true); } - private void getOrCreateCache(boolean isPersistent, long maxWeightInBytes) { + private void createCache(long maxWeightInBytes) { // our EhcacheEventListener should receive events every time an entry is changed CacheEventListenerConfigurationBuilder listenerConfig = CacheEventListenerConfigurationBuilder .newEventListenerConfiguration(listener, @@ -107,30 +113,10 @@ private void getOrCreateCache(boolean isPersistent, long maxWeightInBytes) { .ordered().asynchronous(); // ordered() has some performance penalty as compared to unordered(), we can also use synchronous() - try { - cache = cacheManager.createCache(cacheAlias, - CacheConfigurationBuilder.newCacheConfigurationBuilder( - EhcacheKey.class, BytesReference.class, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(maxWeightInBytes, MemoryUnit.B, isPersistent)) - .withService(listenerConfig)); - } catch (IllegalArgumentException e) { - // Thrown when the cache already exists, which may happen in test cases - // In this case the listener is configured to send messages to some other disk tier instance, which we don't want - // (it was set up unnecessarily by the test case) - - // change config of existing cache to use this listener rather than the one instantiated by the test case - cache = cacheManager.getCache(cacheAlias, EhcacheKey.class, BytesReference.class); - // cache.getRuntimeConfiguration().cacheConfigurationListenerList contains the old listener, but it's private - // and theres no method to clear it unless you have the actual listener object, so it has to stay i think - - cache.getRuntimeConfiguration().registerCacheEventListener(listener, EventOrdering.ORDERED, EventFiring.ASYNCHRONOUS, - EnumSet.of( - EventType.EVICTED, - EventType.EXPIRED, - EventType.REMOVED, - EventType.UPDATED, - EventType.CREATED)); - int k = 1; - } + cache = cacheManager.createCache(cacheAlias, + CacheConfigurationBuilder.newCacheConfigurationBuilder( + EhcacheKey.class, BytesReference.class, ResourcePoolsBuilder.newResourcePoolsBuilder().disk(maxWeightInBytes, MemoryUnit.B, false)) + .withService(listenerConfig)); } @Override @@ -209,7 +195,6 @@ public Iterable keys() { @Override public int count() { - int j = 0; return (int) count.count(); } @@ -230,33 +215,28 @@ public void onRemoval(RemovalNotification e IndicesRequestCache.Key key = tier.convertEhcacheKeyToOriginal(ehcacheKey); removalListener.onRemoval(new RemovalNotification<>(key, oldValue, reason)); } catch (Exception ignored) {} - } } diff --git a/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java b/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java index a38e9d71fb982..16aed8830cf84 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java +++ b/server/src/main/java/org/opensearch/indices/IndicesRequestCache.java @@ -129,10 +129,8 @@ public final class IndicesRequestCache implements TieredCacheEventListener().setOnHeapCachingTier( openSearchOnHeapCache ).setOnDiskCachingTier(diskCachingTier).setTieredCacheEventListener(this).build(); diff --git a/server/src/main/java/org/opensearch/indices/TieredCacheHandler.java b/server/src/main/java/org/opensearch/indices/TieredCacheHandler.java index 0a70375790191..4816f94f7d619 100644 --- a/server/src/main/java/org/opensearch/indices/TieredCacheHandler.java +++ b/server/src/main/java/org/opensearch/indices/TieredCacheHandler.java @@ -23,4 +23,6 @@ public interface TieredCacheHandler { CachingTier getOnHeapCachingTier(); void closeDiskTier(); + + double diskGetTimeMillisEWMA(); } diff --git a/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java b/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java index 6a4aa812cf010..caa4b108946ac 100644 --- a/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java +++ b/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java @@ -147,6 +147,11 @@ public void closeDiskTier() { diskCachingTier.close(); } + @Override + public double diskGetTimeMillisEWMA() { + return diskCachingTier.getTimeMillisEWMA(); + } + public static class CacheValue { V value; TierType source; diff --git a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java index 35da7bac938e6..2bd2bd53a787a 100644 --- a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java +++ b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java @@ -45,19 +45,6 @@ import org.apache.lucene.search.TopDocs; import org.apache.lucene.store.Directory; import org.apache.lucene.util.BytesRef; -import org.ehcache.Cache; -import org.ehcache.PersistentCacheManager; -import org.ehcache.config.builders.CacheConfigurationBuilder; -import org.ehcache.config.builders.CacheEventListenerConfigurationBuilder; -import org.ehcache.config.builders.CacheManagerBuilder; -import org.ehcache.config.builders.PooledExecutionServiceConfigurationBuilder; -import org.ehcache.config.builders.ResourcePoolsBuilder; -import org.ehcache.config.units.MemoryUnit; -import org.ehcache.core.internal.statistics.DefaultStatisticsService; -import org.ehcache.core.spi.service.StatisticsService; -import org.ehcache.core.statistics.TierStatistics; -import org.ehcache.event.EventType; -import org.ehcache.impl.config.executor.PooledExecutionServiceConfiguration; import org.opensearch.common.CheckedSupplier; import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.lucene.index.OpenSearchDirectoryReader; @@ -80,9 +67,7 @@ import java.io.IOException; import java.io.Serializable; -import java.util.ArrayList; import java.util.Arrays; -import java.util.List; import java.util.UUID; import java.util.concurrent.atomic.AtomicBoolean; @@ -196,18 +181,17 @@ public void testSpillover() throws Exception { TestEntity entity = new TestEntity(requestCacheStats, indexShard); Loader loader = new Loader(reader, 0); System.out.println("On-heap cache size at start = " + requestCacheStats.stats().getMemorySizeInBytes()); - IndicesRequestCache.Key[] keys = new IndicesRequestCache.Key[maxNumInHeap + 1]; + BytesReference[] termBytesArr = new BytesReference[maxNumInHeap + 1]; + for (int i = 0; i < maxNumInHeap + 1; i++) { TermQueryBuilder termQuery = new TermQueryBuilder("id", String.valueOf(i)); BytesReference termBytes = XContentHelper.toXContent(termQuery, MediaTypeRegistry.JSON, false); String rKey = ((OpenSearchDirectoryReader) reader).getDelegatingCacheHelper().getDelegatingCacheKey().getId().toString(); - keys[i] = cache.new Key(entity, termBytes, rKey); + termBytesArr[i] = termBytes; BytesReference value = cache.getOrCompute(entity, loader, reader, termBytes); - System.out.println("On-heap cache size after " + (i+1) + " queries = " + requestCacheStats.stats().getMemorySizeInBytes()); - System.out.println("Disk cache size after " + (i+1) + " queries = " + requestCacheStats.stats(TierType.DISK).getMemorySizeInBytes()); } - // attempt to get value from disk cache, the first key should have been evicted - BytesReference firstValue = cache.tieredCacheHandler.get(keys[0]); + // get value from disk cache, the first key should have been evicted + BytesReference firstValue = cache.getOrCompute(entity, loader, reader, termBytesArr[0]); assertEquals(maxNumInHeap * heapKeySize, requestCacheStats.stats().getMemorySizeInBytes()); // TODO: disk weight bytes @@ -219,7 +203,47 @@ public void testSpillover() throws Exception { assertEquals(maxNumInHeap, cache.tieredCacheHandler.count(TierType.ON_HEAP)); assertEquals(1, cache.tieredCacheHandler.count(TierType.DISK)); - // more? + // get a value from heap cache, second key should still be there + BytesReference secondValue = cache.getOrCompute(entity, loader, reader, termBytesArr[1]); + // get the value on disk cache again + BytesReference firstValueAgain = cache.getOrCompute(entity, loader, reader, termBytesArr[0]); + + assertEquals(1, requestCacheStats.stats().getEvictions()); + assertEquals(2, requestCacheStats.stats(TierType.DISK).getHitCount()); + assertEquals(maxNumInHeap + 1, requestCacheStats.stats(TierType.DISK).getMissCount()); + assertEquals(1, requestCacheStats.stats().getHitCount()); + assertEquals(maxNumInHeap + 3, requestCacheStats.stats().getMissCount()); + assertEquals(maxNumInHeap, cache.tieredCacheHandler.count(TierType.ON_HEAP)); + assertEquals(1, cache.tieredCacheHandler.count(TierType.DISK)); + + IOUtils.close(reader, writer, dir, cache); + cache.closeDiskTier(); + } + + public void testDiskGetTimeEWMA() throws Exception { + ShardRequestCache requestCacheStats = new ShardRequestCache(); + Settings.Builder settingsBuilder = Settings.builder(); + long heapSizeBytes = 0; // skip directly to disk cache + settingsBuilder.put("indices.requests.cache.size", new ByteSizeValue(heapSizeBytes)); + IndicesRequestCache cache = new IndicesRequestCache(settingsBuilder.build(), getInstanceFromNode(IndicesService.class)); + + Directory dir = newDirectory(); + IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig()); + writer.addDocument(newDoc(0, "foo")); + DirectoryReader reader = OpenSearchDirectoryReader.wrap(DirectoryReader.open(writer), new ShardId("foo", "bar", 1)); + AtomicBoolean indexShard = new AtomicBoolean(true); + + TestEntity entity = new TestEntity(requestCacheStats, indexShard); + Loader loader = new Loader(reader, 0); + + for (int i = 0; i < 50; i++) { + TermQueryBuilder termQuery = new TermQueryBuilder("id", String.valueOf(i)); + BytesReference termBytes = XContentHelper.toXContent(termQuery, MediaTypeRegistry.JSON, false); + BytesReference value = cache.getOrCompute(entity, loader, reader, termBytes); + // on my machine get time EWMA converges to ~0.025 ms, but it does have an SSD + assertTrue(cache.tieredCacheHandler.diskGetTimeMillisEWMA() > 0); + } + IOUtils.close(reader, writer, dir, cache); cache.closeDiskTier(); } From 7155afd7f651288a3416967b3087d0ff835f0353 Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Tue, 17 Oct 2023 16:16:35 -0700 Subject: [PATCH 20/22] cleanup --- .../java/org/opensearch/indices/EhcacheDiskCachingTier.java | 3 --- .../indices/TieredCacheSpilloverStrategyHandler.java | 3 +-- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java index ce0fc52c9a96e..2469fd5934ea0 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java @@ -15,8 +15,6 @@ import org.ehcache.config.builders.PooledExecutionServiceConfigurationBuilder; import org.ehcache.config.builders.ResourcePoolsBuilder; import org.ehcache.config.units.MemoryUnit; -import org.ehcache.event.EventFiring; -import org.ehcache.event.EventOrdering; import org.ehcache.event.EventType; import org.ehcache.impl.config.executor.PooledExecutionServiceConfiguration; import org.opensearch.common.ExponentiallyWeightedMovingAverage; @@ -29,7 +27,6 @@ import java.io.IOException; import java.util.Collections; -import java.util.EnumSet; public class EhcacheDiskCachingTier implements DiskCachingTier, RemovalListener { diff --git a/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java b/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java index caa4b108946ac..db9d4ffe41447 100644 --- a/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java +++ b/server/src/main/java/org/opensearch/indices/TieredCacheSpilloverStrategyHandler.java @@ -26,7 +26,7 @@ public class TieredCacheSpilloverStrategyHandler implements TieredCacheHandler, RemovalListener { private final OnHeapCachingTier onHeapCachingTier; - private final DiskCachingTier diskCachingTier; // changed for testing + private final DiskCachingTier diskCachingTier; private final TieredCacheEventListener tieredCacheEventListener; /** @@ -37,7 +37,6 @@ public class TieredCacheSpilloverStrategyHandler impleme private TieredCacheSpilloverStrategyHandler( OnHeapCachingTier onHeapCachingTier, DiskCachingTier diskCachingTier, - // changed to EhcacheDiskCachingTier from CachingTier, to enable close() method, which is needed for tests. Implement close() in CachingTier or DiskCachingTier? TieredCacheEventListener tieredCacheEventListener ) { this.onHeapCachingTier = Objects.requireNonNull(onHeapCachingTier); From 8510637aad3f44fbeba200a8e6565c588a0a98e8 Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Wed, 18 Oct 2023 15:26:43 -0700 Subject: [PATCH 21/22] Fixed issue where multiple mock nodes on one JVM (in IT tests) would cause initialization errors in the cache manager --- .../indices/EhcacheDiskCachingTier.java | 48 ++++++++++++++----- .../indices/IndicesRequestCache.java | 2 +- .../opensearch/indices/IndicesService.java | 4 ++ 3 files changed, 42 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java index 2469fd5934ea0..01fe6d491a58a 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java @@ -21,18 +21,35 @@ import org.opensearch.common.cache.RemovalListener; import org.ehcache.Cache; import org.opensearch.common.cache.RemovalNotification; +import org.opensearch.common.io.PathUtils; import org.opensearch.common.metrics.CounterMetric; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.common.io.stream.BytesStreamInput; import java.io.IOException; import java.util.Collections; +import java.util.HashMap; public class EhcacheDiskCachingTier implements DiskCachingTier, RemovalListener { - public static PersistentCacheManager cacheManager; + public static HashMap cacheManagers = new HashMap<>(); + // Because of the way test cases are set up, each node may try to instantiate several disk caching tiers. + // Only one of them will be used, but there will be initialization errors when multiple cache managers try to + // use the same file path and create/get caches with the same alias. We resolve this with a static reference + // to a cache manager, which is populated if it is null and reused if it is not. + // (See https://stackoverflow.com/questions/53756412/ehcache-org-ehcache-statetransitionexception-persistence-directory-already-lo) + // To deal with IT cases, we need to create a manager per node, as otherwise nodes will try to reuse the same manager, + // so we get the correct cache manager by looking up the node ID in this map. + // I don't think any of this can happen in production, because nodes shouldn't share a JVM, + // and they should only instantiate their services once? But it's best to resolve it anyway. + + private PersistentCacheManager cacheManager; // This is the manager this tier will actually use private Cache cache; - public final static String DISK_CACHE_FP = "disk_cache_tier"; // Placeholder. this should probably be defined somewhere else, since we need to change security.policy based on its value + public final static String BASE_DISK_CACHE_FP = "disk_cache_tier"; + // Placeholder. this should probably be defined somewhere else, since we need to change security.policy based on its value + // To accomodate test setups, where multiple nodes may exist on the same filesystem, we add the node ID to the end of this + // These will be subfolders of BASE_DISK_CACHE_FP + private final String diskCacheFP; // the one to use for this node private RemovalListener removalListener; private ExponentiallyWeightedMovingAverage getTimeMillisEWMA; private static final double GET_TIME_EWMA_ALPHA = 0.3; // This is the value used elsewhere in OpenSearch @@ -42,6 +59,7 @@ public class EhcacheDiskCachingTier implements DiskCachingTier().setOnHeapCachingTier( openSearchOnHeapCache ).setOnDiskCachingTier(diskCachingTier).setTieredCacheEventListener(this).build(); diff --git a/server/src/main/java/org/opensearch/indices/IndicesService.java b/server/src/main/java/org/opensearch/indices/IndicesService.java index f5e71327b6e7b..4be7061deabf3 100644 --- a/server/src/main/java/org/opensearch/indices/IndicesService.java +++ b/server/src/main/java/org/opensearch/indices/IndicesService.java @@ -1939,6 +1939,10 @@ public boolean allPendingDanglingIndicesWritten() { || (danglingIndicesToWrite.isEmpty() && danglingIndicesThreadPoolExecutor.getActiveCount() == 0); } + public String getNodeId() { + return nodeEnv.nodeId(); + } + /** * Validates the cluster default index refresh interval. * From 16016444b4c00030074af6d59d36301951347aab Mon Sep 17 00:00:00 2001 From: Peter Alfonsi Date: Thu, 19 Oct 2023 13:56:41 -0700 Subject: [PATCH 22/22] Added concurrency test, which fails possibly due to outdated framework --- .../indices/DummySerializableKey.java | 50 ----------- .../indices/EhcacheDiskCachingTier.java | 8 +- .../TieredCacheSpilloverStrategyHandler.java | 26 +++++- .../indices/IndicesRequestCacheTests.java | 83 +++++++++++++++++++ 4 files changed, 114 insertions(+), 53 deletions(-) delete mode 100644 server/src/main/java/org/opensearch/indices/DummySerializableKey.java diff --git a/server/src/main/java/org/opensearch/indices/DummySerializableKey.java b/server/src/main/java/org/opensearch/indices/DummySerializableKey.java deleted file mode 100644 index 7f2888f6e65f7..0000000000000 --- a/server/src/main/java/org/opensearch/indices/DummySerializableKey.java +++ /dev/null @@ -1,50 +0,0 @@ -/* - * 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.indices; - -import java.io.Serializable; -import java.util.Objects; - -public class DummySerializableKey implements Serializable { - private Integer i; - private String s; - public DummySerializableKey(Integer i, String s) { - this.i = i; - this.s = s; - } - - public int getI() { - return i; - } - public String getS() { - return s; - } - @Override - public boolean equals(Object o) { - if (o == this) { - return true; - } - if (!(o instanceof DummySerializableKey)) { - return false; - } - DummySerializableKey other = (DummySerializableKey) o; - return Objects.equals(this.i, other.i) && this.s.equals(other.s); - } - @Override - public final int hashCode() { - int result = 11; - if (i != null) { - result = 31 * result + i.hashCode(); - } - if (s != null) { - result = 31 * result + s.hashCode(); - } - return result; - } -} diff --git a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java index 01fe6d491a58a..6ccbb68515b50 100644 --- a/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java +++ b/server/src/main/java/org/opensearch/indices/EhcacheDiskCachingTier.java @@ -60,6 +60,7 @@ public class EhcacheDiskCachingTier implements DiskCachingTier impleme */ private final List> cachingTierList; + /*public AtomicInteger numGets; // debug only + public AtomicInteger numHeapGets; + public AtomicInteger numHeapHits; + public AtomicInteger numDiskHits;*/ private TieredCacheSpilloverStrategyHandler( OnHeapCachingTier onHeapCachingTier, DiskCachingTier diskCachingTier, @@ -43,6 +48,10 @@ private TieredCacheSpilloverStrategyHandler( this.diskCachingTier = Objects.requireNonNull(diskCachingTier); this.tieredCacheEventListener = tieredCacheEventListener; this.cachingTierList = Arrays.asList(onHeapCachingTier, diskCachingTier); + /*this.numGets = new AtomicInteger(); + this.numHeapGets = new AtomicInteger(); + this.numHeapHits = new AtomicInteger(); + this.numDiskHits = new AtomicInteger();*/ setRemovalListeners(); } @@ -118,8 +127,8 @@ public CachingTier getOnHeapCachingTier() { return this.onHeapCachingTier; } - public DiskCachingTier getDiskCachingTier() { // change to CachingTier after debug - return this.diskCachingTier; + public EhcacheDiskCachingTier getDiskCachingTier() { // change to CachingTier after debug + return (EhcacheDiskCachingTier) this.diskCachingTier; } private void setRemovalListeners() { @@ -131,9 +140,22 @@ private void setRemovalListeners() { private Function> getValueFromTierCache() { return key -> { for (CachingTier cachingTier : cachingTierList) { + // counters are debug only + /*if (cachingTier.getTierType() == TierType.ON_HEAP) { + numHeapGets.incrementAndGet(); + } else if (cachingTier.getTierType() == TierType.DISK) { + numGets.incrementAndGet(); + }*/ + V value = cachingTier.get(key); if (value != null) { tieredCacheEventListener.onHit(key, value, cachingTier.getTierType()); + /*if (cachingTier.getTierType() == TierType.ON_HEAP) { + numHeapHits.incrementAndGet(); + } + if (cachingTier.getTierType() == TierType.DISK) { + numDiskHits.incrementAndGet(); + }*/ return new CacheValue<>(value, cachingTier.getTierType()); } tieredCacheEventListener.onMiss(key, cachingTier.getTierType()); diff --git a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java index 2bd2bd53a787a..a68f0795c2e2c 100644 --- a/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java +++ b/server/src/test/java/org/opensearch/indices/IndicesRequestCacheTests.java @@ -46,6 +46,7 @@ import org.apache.lucene.store.Directory; import org.apache.lucene.util.BytesRef; import org.opensearch.common.CheckedSupplier; +import org.opensearch.common.Randomness; import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.lucene.index.OpenSearchDirectoryReader; import org.opensearch.common.settings.Settings; @@ -67,8 +68,13 @@ import java.io.IOException; import java.io.Serializable; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Random; import java.util.UUID; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.atomic.AtomicBoolean; public class IndicesRequestCacheTests extends OpenSearchSingleNodeTestCase { @@ -248,6 +254,83 @@ public void testDiskGetTimeEWMA() throws Exception { cache.closeDiskTier(); } + public void testEhcacheConcurrency() throws Exception { + ShardRequestCache requestCacheStats = new ShardRequestCache(); + Settings.Builder settingsBuilder = Settings.builder(); + long heapSizeBytes = 0; // skip directly to disk cache + settingsBuilder.put("indices.requests.cache.size", new ByteSizeValue(heapSizeBytes)); + IndicesRequestCache cache = new IndicesRequestCache(settingsBuilder.build(), getInstanceFromNode(IndicesService.class)); + + Directory dir = newDirectory(); + IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig()); + writer.addDocument(newDoc(0, "foo")); + DirectoryReader reader = OpenSearchDirectoryReader.wrap(DirectoryReader.open(writer), new ShardId("foo", "bar", 1)); + AtomicBoolean indexShard = new AtomicBoolean(true); + + TestEntity entity = new TestEntity(requestCacheStats, indexShard); + Loader loader = new Loader(reader, 0); + + Random rand = Randomness.get(); + int minThreads = 6; + int maxThreads = 8; + int numThreads = rand.nextInt(maxThreads - minThreads) + minThreads; + ThreadPoolExecutor executor = (ThreadPoolExecutor) Executors.newFixedThreadPool(numThreads); + int numRequests = 50; + int numRepeats = 10; + BytesReference[] termBytesArr = new BytesReference[numRequests]; + ArrayList permutation = new ArrayList<>(); + + // Have these threads make 50 requests, with 10 repeats, in random order, and keep track of the keys. + // At the end, make sure that all the keys are in the cache, there are 40 misses, and 10 hits. + + for (int i = 0; i < numRequests; i++) { + int searchValue = i; + if (i > numRequests - numRepeats - 1) { + searchValue = i - (numRequests - numRepeats); // repeat values 0-9 + } + //System.out.println("values: " + i + " " + searchValue); + permutation.add(searchValue); + if (i == searchValue) { + TermQueryBuilder termQuery = new TermQueryBuilder("id", String.valueOf(searchValue)); + BytesReference termBytes = XContentHelper.toXContent(termQuery, MediaTypeRegistry.JSON, false); + termBytesArr[i] = termBytes; + } + } + java.util.Collections.shuffle(permutation); + + ArrayList> futures = new ArrayList<>(); + + for (int j = 0; j < permutation.size(); j++) { + int keyNumber = permutation.get(j); + Future fut = executor.submit(() -> cache.getOrCompute(entity, loader, reader, termBytesArr[keyNumber])); + futures.add(fut); + } + + // now go thru and get them all + for (Future fut : futures) { + BytesReference value = fut.get(); + assertNotNull(value); + } + + System.out.println("heap size " + cache.tieredCacheHandler.count(TierType.ON_HEAP)); + System.out.println("disk size " + cache.tieredCacheHandler.count(TierType.DISK)); + System.out.println("disk misses " + requestCacheStats.stats(TierType.DISK).getMissCount()); + System.out.println("disk hits " + requestCacheStats.stats(TierType.DISK).getHitCount()); + /*System.out.println("disk num gets " + cache.tieredCacheHandler.getDiskCachingTier().numGets); + System.out.println("handler num get " + cache.tieredCacheHandler.numGets.intValue()); + System.out.println("handler num heap get " + cache.tieredCacheHandler.numHeapGets.intValue()); + System.out.println("handler num heap hit " + cache.tieredCacheHandler.numHeapHits.intValue()); + System.out.println("handler num disk hit " + cache.tieredCacheHandler.numDiskHits.intValue());*/ + + assertEquals(numRequests - numRepeats, cache.tieredCacheHandler.count(TierType.DISK)); // correct + assertEquals(numRequests - numRepeats, requestCacheStats.stats(TierType.DISK).getMissCount()); // usually correctly 40, sometimes 41 + assertEquals(numRepeats, requestCacheStats.stats(TierType.DISK).getHitCount()); // should be 10, is usually 9 + + IOUtils.close(reader, writer, dir, cache); + cache.closeDiskTier(); + + } + public void testCacheDifferentReaders() throws Exception { IndicesRequestCache cache = new IndicesRequestCache(Settings.EMPTY, getInstanceFromNode(IndicesService.class)); AtomicBoolean indexShard = new AtomicBoolean(true);