You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by GitBox <gi...@apache.org> on 2022/03/21 13:55:20 UTC
[GitHub] [jackrabbit-oak] klcodanr opened a new pull request #524: OAK-9727 - ElasticSearch Index Function Support
klcodanr opened a new pull request #524:
URL: https://github.com/apache/jackrabbit-oak/pull/524
Most of the changes are to correct issues with the tests.
The primary changes are to introduce a delay between the test changes and test execution to ensure that the Elastic index is updated by the time the assertions are run in the [FunctionIndexCommonTest.java](https://github.com/apache/jackrabbit-oak/compare/trunk...klcodanr:elastic-function-support?expand=1#diff-2d0a75bdc45709baff8c41f725c939f9f37e9414752b37ad03e335c2d40b33c7). This was implemented as an overridden method so that the Lucene indexing does not have any delays. (see postCommitHook)
The other change is to ensure that the functional restrictions are translated into Document properties when creating the [ElasticIndexDefinition.java](https://github.com/apache/jackrabbit-oak/compare/trunk...klcodanr:elastic-function-support?expand=1#diff-c047e881bb3fbf15e48f90204a88d5ac9b8f10c3ac6310098e40151a111f836d) and including them in the [ElasticIndexPlanner.java](https://github.com/apache/jackrabbit-oak/compare/trunk...klcodanr:elastic-function-support?expand=1#diff-747aaeec4a6e472c51175546720cc83729ce63ee0e44af12911a36b6e35b8d4c).
I did also reduce the re-indexing delay in the [ElasticTestRepositoryBuilder.java](https://github.com/apache/jackrabbit-oak/compare/trunk...klcodanr:elastic-function-support?expand=1#diff-102e564f471507afd5abb0fb9715a26201b9a46f2f3a3d9588a1fd1e9428e4fa) to reduce the impact of waiting for the changes to be indexed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-oak] thomasmueller commented on a change in pull request #524: OAK-9727 - ElasticSearch Index Function Support
Posted by GitBox <gi...@apache.org>.
thomasmueller commented on a change in pull request #524:
URL: https://github.com/apache/jackrabbit-oak/pull/524#discussion_r831235065
##########
File path: oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexDefinition.java
##########
@@ -133,9 +135,16 @@ public ElasticIndexDefinition(NodeState root, NodeState defn, String indexPath,
this.propertiesByName = getDefinedRules()
.stream()
- .flatMap(rule -> StreamSupport.stream(rule.getProperties().spliterator(), false))
+ .flatMap(rule -> Stream.concat(StreamSupport.stream(rule.getProperties().spliterator(), false),
+ rule.getFunctionRestrictions().stream()))
.filter(pd -> pd.index) // keep only properties that can be indexed
- .collect(Collectors.groupingBy(pd -> pd.name));
+ .collect(Collectors.groupingBy(pd -> {
+ if(pd.function != null){
Review comment:
Nit: Please add spaces around ().
##########
File path: oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticFunctionIndexCommonTest.java
##########
@@ -16,42 +16,61 @@
*/
package org.apache.jackrabbit.oak.plugins.index.elastic;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NODE_TYPE;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.REINDEX_PROPERTY_NAME;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.TYPE_PROPERTY_NAME;
+
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
import org.apache.jackrabbit.JcrConstants;
+import org.apache.jackrabbit.oak.InitialContentHelper;
import org.apache.jackrabbit.oak.api.ContentRepository;
import org.apache.jackrabbit.oak.api.Tree;
import org.apache.jackrabbit.oak.api.Type;
import org.apache.jackrabbit.oak.plugins.index.FunctionIndexCommonTest;
+import org.apache.jackrabbit.oak.plugins.index.elastic.index.ElasticIndexEditor;
import org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants;
+import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeStore;
import org.apache.jackrabbit.oak.plugins.memory.PropertyStates;
import org.junit.ClassRule;
-import org.junit.Ignore;
-
-import java.util.Set;
-import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME;
-import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NODE_TYPE;
-import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.REINDEX_PROPERTY_NAME;
-import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.TYPE_PROPERTY_NAME;
-
-@Ignore
public class ElasticFunctionIndexCommonTest extends FunctionIndexCommonTest {
@ClassRule
- public static final ElasticConnectionRule elasticRule =
- new ElasticConnectionRule(ElasticTestUtils.ELASTIC_CONNECTION_STRING);
+ public static final ElasticConnectionRule elasticRule = new ElasticConnectionRule(
+ ElasticTestUtils.ELASTIC_CONNECTION_STRING);
public ElasticFunctionIndexCommonTest() {
indexOptions = new ElasticIndexOptions();
}
+ @Override
+ protected String getIndexProvider() {
+ return "elasticsearch:";
+ }
+
+ @Override
+ protected void postCommitHook() {
+ try {
+ TimeUnit.SECONDS.sleep(2);
Review comment:
Of course that's not that great... I assume we can't use synchronous indexes, and can't commit changes to Elasticsearch explicitly...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-oak] fabriziofortino commented on a change in pull request #524: OAK-9727 - ElasticSearch Index Function Support
Posted by GitBox <gi...@apache.org>.
fabriziofortino commented on a change in pull request #524:
URL: https://github.com/apache/jackrabbit-oak/pull/524#discussion_r831289792
##########
File path: oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticFunctionIndexCommonTest.java
##########
@@ -16,42 +16,61 @@
*/
package org.apache.jackrabbit.oak.plugins.index.elastic;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NODE_TYPE;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.REINDEX_PROPERTY_NAME;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.TYPE_PROPERTY_NAME;
+
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
import org.apache.jackrabbit.JcrConstants;
+import org.apache.jackrabbit.oak.InitialContentHelper;
import org.apache.jackrabbit.oak.api.ContentRepository;
import org.apache.jackrabbit.oak.api.Tree;
import org.apache.jackrabbit.oak.api.Type;
import org.apache.jackrabbit.oak.plugins.index.FunctionIndexCommonTest;
+import org.apache.jackrabbit.oak.plugins.index.elastic.index.ElasticIndexEditor;
import org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants;
+import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeStore;
import org.apache.jackrabbit.oak.plugins.memory.PropertyStates;
import org.junit.ClassRule;
-import org.junit.Ignore;
-
-import java.util.Set;
-import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME;
-import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NODE_TYPE;
-import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.REINDEX_PROPERTY_NAME;
-import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.TYPE_PROPERTY_NAME;
-
-@Ignore
public class ElasticFunctionIndexCommonTest extends FunctionIndexCommonTest {
@ClassRule
- public static final ElasticConnectionRule elasticRule =
- new ElasticConnectionRule(ElasticTestUtils.ELASTIC_CONNECTION_STRING);
+ public static final ElasticConnectionRule elasticRule = new ElasticConnectionRule(
+ ElasticTestUtils.ELASTIC_CONNECTION_STRING);
public ElasticFunctionIndexCommonTest() {
indexOptions = new ElasticIndexOptions();
}
+ @Override
+ protected String getIndexProvider() {
+ return "elasticsearch:";
+ }
+
+ @Override
+ protected void postCommitHook() {
+ try {
+ TimeUnit.SECONDS.sleep(2);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ }
+ }
+
Review comment:
We should not use this approach. We were using something similar in the past but tests were flaky.
As an alternative, I would remove the `postCommitHook` method and use `TestUtils.assertEventually` in the tests (see `IndexSuggestionCommonTest` for an example).
##########
File path: oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexDefinition.java
##########
@@ -18,21 +18,23 @@
*/
package org.apache.jackrabbit.oak.plugins.index.elastic;
-import org.apache.jackrabbit.oak.api.Type;
-import org.apache.jackrabbit.oak.plugins.index.search.IndexDefinition;
-import org.apache.jackrabbit.oak.plugins.index.search.PropertyDefinition;
-import org.apache.jackrabbit.oak.spi.state.NodeState;
-import org.jetbrains.annotations.NotNull;
+import static org.apache.jackrabbit.oak.plugins.index.search.util.ConfigUtil.getOptionalValue;
+import static org.apache.jackrabbit.oak.plugins.index.search.util.ConfigUtil.getOptionalValues;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import java.util.stream.Collectors;
+import java.util.stream.Stream;
import java.util.stream.StreamSupport;
-import static org.apache.jackrabbit.oak.plugins.index.search.util.ConfigUtil.getOptionalValue;
-import static org.apache.jackrabbit.oak.plugins.index.search.util.ConfigUtil.getOptionalValues;
+import org.apache.jackrabbit.oak.api.Type;
+import org.apache.jackrabbit.oak.plugins.index.search.FieldNames;
Review comment:
unused import
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-oak] thomasmueller merged pull request #524: OAK-9727 - ElasticSearch Index Function Support
Posted by GitBox <gi...@apache.org>.
thomasmueller merged pull request #524:
URL: https://github.com/apache/jackrabbit-oak/pull/524
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jackrabbit-oak] klcodanr commented on a change in pull request #524: OAK-9727 - ElasticSearch Index Function Support
Posted by GitBox <gi...@apache.org>.
klcodanr commented on a change in pull request #524:
URL: https://github.com/apache/jackrabbit-oak/pull/524#discussion_r831325838
##########
File path: oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticFunctionIndexCommonTest.java
##########
@@ -16,42 +16,61 @@
*/
package org.apache.jackrabbit.oak.plugins.index.elastic;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NODE_TYPE;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.REINDEX_PROPERTY_NAME;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.TYPE_PROPERTY_NAME;
+
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
import org.apache.jackrabbit.JcrConstants;
+import org.apache.jackrabbit.oak.InitialContentHelper;
import org.apache.jackrabbit.oak.api.ContentRepository;
import org.apache.jackrabbit.oak.api.Tree;
import org.apache.jackrabbit.oak.api.Type;
import org.apache.jackrabbit.oak.plugins.index.FunctionIndexCommonTest;
+import org.apache.jackrabbit.oak.plugins.index.elastic.index.ElasticIndexEditor;
import org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants;
+import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeStore;
import org.apache.jackrabbit.oak.plugins.memory.PropertyStates;
import org.junit.ClassRule;
-import org.junit.Ignore;
-
-import java.util.Set;
-import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME;
-import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NODE_TYPE;
-import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.REINDEX_PROPERTY_NAME;
-import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.TYPE_PROPERTY_NAME;
-
-@Ignore
public class ElasticFunctionIndexCommonTest extends FunctionIndexCommonTest {
@ClassRule
- public static final ElasticConnectionRule elasticRule =
- new ElasticConnectionRule(ElasticTestUtils.ELASTIC_CONNECTION_STRING);
+ public static final ElasticConnectionRule elasticRule = new ElasticConnectionRule(
+ ElasticTestUtils.ELASTIC_CONNECTION_STRING);
public ElasticFunctionIndexCommonTest() {
indexOptions = new ElasticIndexOptions();
}
+ @Override
+ protected String getIndexProvider() {
+ return "elasticsearch:";
+ }
+
+ @Override
+ protected void postCommitHook() {
+ try {
+ TimeUnit.SECONDS.sleep(2);
Review comment:
From my investigation while working on this, I didn't see a good way to either force the index to update or to just perform indexing asynchronously. I opened another ticket to track improving the performance of this test:
https://issues.apache.org/jira/browse/OAK-9733
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org