Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[BUG] GeoPointShapeQueryTests.testQueryLinearRing fails with UnsupportedOperationException #11688

Closed
rishabhmaurya opened this issue Dec 28, 2023 · 13 comments · Fixed by #12783 or #12834
Closed
Labels
bug Something isn't working flaky-test Random test failure that succeeds on second run Geospatial Search Search query, autocomplete ...etc v2.14.0

Comments

@rishabhmaurya
Copy link
Contributor

rishabhmaurya commented Dec 28, 2023

Describe the bug

GeoPointShapeQueryTests.testQueryLinearRing fails with UnsupportedOperationException -

Here is the failed test - https://build.ci.opensearch.org/job/gradle-check/31539/testReport/junit/org.opensearch.search.geo/GeoPointShapeQueryTests/testQueryLinearRing/

The failure is not reproducible on mac, amazonlinux and ubuntu when I tried to reproduce them locally with the same seed.

Relates to PR - #11039

java.lang.UnsupportedOperationException: line ring cannot be serialized using GeoJson
	at __randomizedtesting.SeedInfo.seed([D24747E5E9B452AE:CFF1D9D2D64D1980]:0)
	at org.opensearch.common.geo.GeoJson$3.visit(GeoJson.java:561)
	at org.opensearch.common.geo.GeoJson$3.visit(GeoJson.java:543)
	at org.opensearch.geometry.LinearRing.visit(LinearRing.java:84)
	at org.opensearch.common.geo.GeoJson.getGeoJsonName(GeoJson.java:543)
	at org.opensearch.common.geo.GeoJson.toXContent(GeoJson.java:103)
	at org.opensearch.index.query.AbstractGeometryQueryBuilder.doXContent(AbstractGeometryQueryBuilder.java:452)
	at org.opensearch.index.query.AbstractQueryBuilder.toXContent(AbstractQueryBuilder.java:101)
	at org.opensearch.core.xcontent.XContentBuilder.value(XContentBuilder.java:887)
	at org.opensearch.core.xcontent.XContentBuilder.value(XContentBuilder.java:880)
	at org.opensearch.core.xcontent.XContentBuilder.field(XContentBuilder.java:872)
	at org.opensearch.search.builder.SearchSourceBuilder.innerToXContent(SearchSourceBuilder.java:1350)
	at org.opensearch.search.builder.SearchSourceBuilder.toXContent(SearchSourceBuilder.java:1490)
	at org.opensearch.core.xcontent.XContentHelper.toXContent(XContentHelper.java:46)
	at org.opensearch.search.builder.SearchSourceBuilder.toString(SearchSourceBuilder.java:1810)
	at org.opensearch.action.search.SearchRequest.buildDescription(SearchRequest.java:715)
	at org.opensearch.action.search.SearchTask.getDescription(SearchTask.java:83)
	at org.opensearch.tasks.TaskManager.register(TaskManager.java:210)
	at org.opensearch.action.support.TransportAction.execute(TransportAction.java:99)
	at org.opensearch.client.node.NodeClient.executeLocally(NodeClient.java:110)
	at org.opensearch.client.node.NodeClient.doExecute(NodeClient.java:97)
	at org.opensearch.client.support.AbstractClient.execute(AbstractClient.java:476)
	at org.opensearch.client.support.AbstractClient.execute(AbstractClient.java:463)
	at org.opensearch.action.ActionRequestBuilder.execute(ActionRequestBuilder.java:66)
	at org.opensearch.action.ActionRequestBuilder.get(ActionRequestBuilder.java:73)
	at org.opensearch.search.geo.GeoPointShapeQueryTests.testQueryLinearRing(GeoPointShapeQueryTests.java:140)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at com.carrotsearch.randomizedtesting.RandomizedRunner.invoke(RandomizedRunner.java:1750)
	at com.carrotsearch.randomizedtesting.RandomizedRunner$8.evaluate(RandomizedRunner.java:938)
	at com.carrotsearch.randomizedtesting.RandomizedRunner$9.evaluate(RandomizedRunner.java:974)
	at com.carrotsearch.randomizedtesting.RandomizedRunner$10.evaluate(RandomizedRunner.java:988)
	at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	at org.apache.lucene.tests.util.TestRuleSetupTeardownChained$1.evaluate(TestRuleSetupTeardownChained.java:48)
	at org.apache.lucene.tests.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:43)
	at org.apache.lucene.tests.util.TestRuleThreadAndTestName$1.evaluate(TestRuleThreadAndTestName.java:45)
	at org.apache.lucene.tests.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:60)
	at org.apache.lucene.tests.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:44)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
	at com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:368)
	at com.carrotsearch.randomizedtesting.ThreadLeakControl.forkTimeoutingTask(ThreadLeakControl.java:817)
	at com.carrotsearch.randomizedtesting.ThreadLeakControl$3.evaluate(ThreadLeakControl.java:468)
	at com.carrotsearch.randomizedtesting.RandomizedRunner.runSingleTest(RandomizedRunner.java:947)
	at com.carrotsearch.randomizedtesting.RandomizedRunner$5.evaluate(RandomizedRunner.java:832)
	at com.carrotsearch.randomizedtesting.RandomizedRunner$6.evaluate(RandomizedRunner.java:883)
	at com.carrotsearch.randomizedtesting.RandomizedRunner$7.evaluate(RandomizedRunner.java:894)
	at org.apache.lucene.tests.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:43)
	at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
	at org.apache.lucene.tests.util.TestRuleStoreClassName$1.evaluate(TestRuleStoreClassName.java:38)
	at com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:40)
	at com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:40)
	at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
	at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
	at org.apache.lucene.tests.util.TestRuleAssertionsRequired$1.evaluate(TestRuleAssertionsRequired.java:53)
	at org.apache.lucene.tests.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:43)
	at org.apache.lucene.tests.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:44)
	at org.apache.lucene.tests.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:60)
	at org.apache.lucene.tests.util.TestRuleIgnoreTestSuites$1.evaluate(TestRuleIgnoreTestSuites.java:47)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
	at com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:368)
	at java.base/java.lang.Thread.run(Thread.java:1583)
	Suppressed: java.lang.IllegalStateException: Failed to close the XContentBuilder
		at org.opensearch.core.xcontent.XContentBuilder.close(XContentBuilder.java:1044)
		at org.opensearch.core.xcontent.XContentHelper.toXContent(XContentHelper.java:41)
		... 50 more
	Caused by: java.io.IOException: Unclosed object or array found
		at org.opensearch.common.xcontent.json.JsonXContentGenerator.close(JsonXContentGenerator.java:494)
		at org.opensearch.core.xcontent.XContentBuilder.close(XContentBuilder.java:1042)
		... 51 more

Related component

Build

To Reproduce

Here are the steps to reproduce it, although its not reproducible -

https://build.ci.opensearch.org/job/gradle-check/31539/testReport/junit/org.opensearch.search.geo/GeoPointShapeQueryTests/testQueryLinearRing/

./gradlew ':server:test' --tests "org.opensearch.search.geo.GeoPointShapeQueryTests.testQueryLinearRing" -Dtests.seed=E6A53B1422E4284A -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=hu-HU -Dtests.timezone=Pacific/Funafuti -Druntime.java=21

Expected behavior

Ideally such serialization/deserialization failures should be deterministic not be environment dependent.

Additional Details

Plugins
Please list all plugins currently enabled.

Screenshots
If applicable, add screenshots to help explain your problem.

Host/Environment (please complete the following information):

  • OS: [e.g. iOS]
  • Version [e.g. 22]

Additional context
Add any other context about the problem here.

@rishabhmaurya rishabhmaurya added bug Something isn't working untriaged labels Dec 28, 2023
@github-actions github-actions bot added the Build Build Tasks/Gradle Plugin, groovy scripts, build tools, Javadoc enforcement. label Dec 28, 2023
@rishabhmaurya rishabhmaurya added flaky-test Random test failure that succeeds on second run and removed Build Build Tasks/Gradle Plugin, groovy scripts, build tools, Javadoc enforcement. labels Dec 28, 2023
@rishabhmaurya
Copy link
Contributor Author

Found that it fails here -

logger.trace("register {} [{}] [{}] [{}]", task.getId(), type, action, task.getDescription());

So when trace logs are enabled and it goes to line 210 and calls task.getDescription() - it results in following exception. When trace logs are not enabled, this line never gets called and tests executes normally.

java.lang.UnsupportedOperationException: line ring cannot be serialized using GeoJson
	at __randomizedtesting.SeedInfo.seed([D24747E5E9B452AE:CFF1D9D2D64D1980]:0)
	at org.opensearch.common.geo.GeoJson$3.visit(GeoJson.java:561)
	at org.opensearch.common.geo.GeoJson$3.visit(GeoJson.java:543)
	at org.opensearch.geometry.LinearRing.visit(LinearRing.java:84)
	at org.opensearch.common.geo.GeoJson.getGeoJsonName(GeoJson.java:543)
	at org.opensearch.common.geo.GeoJson.toXContent(GeoJson.java:103)
	at org.opensearch.index.query.AbstractGeometryQueryBuilder.doXContent(AbstractGeometryQueryBuilder.java:452)
	at org.opensearch.index.query.AbstractQueryBuilder.toXContent(AbstractQueryBuilder.java:101)
	at org.opensearch.core.xcontent.XContentBuilder.value(XContentBuilder.java:887)
	at org.opensearch.core.xcontent.XContentBuilder.value(XContentBuilder.java:880)
	at org.opensearch.core.xcontent.XContentBuilder.field(XContentBuilder.java:872)
	at org.opensearch.search.builder.SearchSourceBuilder.innerToXContent(SearchSourceBuilder.java:1350)
	at org.opensearch.search.builder.SearchSourceBuilder.toXContent(SearchSourceBuilder.java:1490)
	at org.opensearch.core.xcontent.XContentHelper.toXContent(XContentHelper.java:46)
	at org.opensearch.search.builder.SearchSourceBuilder.toString(SearchSourceBuilder.java:1810)
	at org.opensearch.action.search.SearchRequest.buildDescription(SearchRequest.java:715)
	at org.opensearch.action.search.SearchTask.getDescription(SearchTask.java:83)
	at org.opensearch.tasks.TaskManager.register(TaskManager.java:210)

@vikasvb90 vikasvb90 added the Search Search query, autocomplete ...etc label Jan 30, 2024
@peternied
Copy link
Member

[Triage - attendees 1 2 3 4 5 6 7 8]
@rishabhmaurya Thanks for filing.

@andrross It sounds like there are more details to this issue, could you update the description to better fit what should be addressed.

@shwetathareja
Copy link
Member

PR - #8992 observed same failure here - https://build.ci.opensearch.org/job/gradle-check/35326/

@shwetathareja
Copy link
Member

#12252 (comment)

@shwetathareja
Copy link
Member

This test seems like a serial offender, causing gradle check across multiple PRs to fail. can this be muted @andrross ?

@rishabhmaurya
Copy link
Contributor Author

Or we can remove -
task.getDescription() from logger.trace("register {} [{}] [{}] [{}]", task.getId(), type, action, task.getDescription()); to fix it. Not sure how viable this option is.

@shwetathareja
Copy link
Member

Or we can remove -
task.getDescription() from logger.trace("register {} [{}] [{}] [{}]", task.getId(), type, action, task.getDescription()); to fix it. Not sure how viable this option is.

@rishabhmaurya : It wouldn't be preferred as it is a trace log and may be useful in the context of debugging for other type of tasks. We should fix the test.

@shwetathareja
Copy link
Member

shwetathareja commented Mar 20, 2024

As @rishabhmaurya pointed out, right now it is failing when trace logging is enabled.
This would be consistent failure when trace is enabled, so we need to check during gradle-check why some runs are with trace enabled and while others are not.

I looked into the test, it is a negative and it expects this same exception, just that from a different flow, but when trace logging is enabled, that exception is pre-empted via task.getDescription() call

    public void testQueryLinearRing() throws Exception {
        XContentBuilder xcb = createDefaultMapping();
        client().admin().indices().prepareCreate("test").setMapping(xcb).get();
        ensureGreen();

        LinearRing linearRing = new LinearRing(new double[] { -25, -35, -25 }, new double[] { -25, -35, -25 });

        try {
            // LinearRing extends Line implements Geometry: expose the build process
            GeoShapeQueryBuilder queryBuilder = new GeoShapeQueryBuilder(defaultGeoFieldName, linearRing);
            SearchRequestBuilder searchRequestBuilder = new SearchRequestBuilder(client(), SearchAction.INSTANCE);
            searchRequestBuilder.setQuery(queryBuilder);
            searchRequestBuilder.setIndices("test");
            searchRequestBuilder.get();       --------->  // here it fails during search task register before it could start the execution, which would have thrown the same exception 
        } catch (SearchPhaseExecutionException e) {
            assertThat(
                e.getCause().getMessage(),
                containsString("Field [" + defaultGeoFieldName + "] does not support LINEARRING queries")
            );
        }
    }

I don't think we can control the logging behavior in TaskManager as it is generic across all tasks.
Our options to fix the test

  1. Expect UnsupportedOperationException in the test and assert on it as well
  2. Change log level for this test? What if somebody introduces another log at debug, this would start to fail again

I would prefer Option 1.
@peternied / Others : Please feel free to share your thoughts if you think there is a better way to handle it.

@shwetathareja
Copy link
Member

Raised the PR to fix it - #12783

@dblock
Copy link
Member

dblock commented Mar 20, 2024

The fix in #12783 doesn't look right, reopening so we don't lose it, cc: @Bukhtawar @shwetathareja (#12783 (comment))

@peternied
Copy link
Member

Does anyone know why is a LinearRing should not be representable in OpenSearch? From the GeoJSON RFC [1] its a polygon... seems easier for our users (and test cases) by representing it as a polygon when outputted.

@rishabhmaurya
Copy link
Contributor Author

rishabhmaurya commented Mar 21, 2024

@andrross @shwetathareja how about this fix - #12844 ?
And we should still address - #12324 independently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working flaky-test Random test failure that succeeds on second run Geospatial Search Search query, autocomplete ...etc v2.14.0
Projects
Status: Done
8 participants