Improve test times for tests using `RandomObjects::addFields` #31556

sohaibiftikhar · 2018-06-25T13:24:45Z

Currently RandomObjects::addFields can potentially generate a large number of fields. This commit decreases the chances that a new object or array is added as a new branch of an object, which lowers the probability of ending up with very big documents generated.

It also reduces the number of documents generated for the SimulatePipelineResponseTests from 10 to 5 to reduce the testing time required for parsing.

-- reduce number of documents generated for SimulatePipelineResponse

javanna · 2018-06-25T13:33:23Z

add to whitelist

sohaibiftikhar · 2018-06-25T13:35:37Z

test/framework/src/main/java/org/elasticsearch/test/RandomObjects.java

-        int numFields = randomIntBetween(random, minNumFields, 10);
+    private static int addFields(Random random, XContentBuilder builder, int minNumFields, int currentDepth,
+                                 int currentFields) throws IOException {
+        int maxTotalFields = 200;


I'm not sure what value is good for this. From my results, 200 seemed to give decently big documents without going beyond 100KB.

I don't think that we need such big docs, 100kb seems a lot still even if it's the upper bound.

javanna

left a question/thought

javanna · 2018-06-25T13:37:59Z

test/framework/src/main/java/org/elasticsearch/test/RandomObjects.java

+        int maxFields = Math.max(minNumFields, 10 - (currentFields * 10)/maxTotalFields); // Map to range 0-10
+        int numFields = randomIntBetween(random, minNumFields, maxFields);
+        int maxDepth = 5 - (currentFields * 5)/maxTotalFields; // Map to range 0-5
+        currentFields += numFields;


with the goal of trying to keep things simple, I wonder if decreasing the max number of fields e.g. to 5 would be enough. What do you think?

Could be. Will need to test this for the average case. The potentially maximum number of fields would still be more than 3000.

maybe we should also decrease the chance that we add another child object compared to leaf fields.

I didn't quite understand. We, already have a check on the depth which takes care of this? Or do you mean something else?

yes we have a check, but I think we end up adding more objects and arrays than we need.

if (currentDepth < 5 && random.nextBoolean()) {

the above if (taken from addFields method) means that we add a new object or array if maxDepth allows, one out of two times. Maybe we want to lower that probability?

Ah okay. I will try to lower the probability based on current depth. What did you mean by leaf fields? The neighbours?

you can also do something similar to what LuceneTestCase#rarely does. we can do it regardless depth to keep things simple?

@javanna I simplified it now. So that max fields is 5 and the probability is 30%. It seems to give similar results in terms of timing.

elasticmachine · 2018-06-25T13:41:10Z

Pinging @elastic/es-core-infra

javanna

LGTM thanks a lot for improving this @sohaibiftikhar !!!

Currently RandomObjects::addFields can potentially generate a large number of fields This commit decreases the chances that a new object or array is added as a new branch of an object, which lowers the probability of ending up with very big documents generated. It also reduces the number of documents generated for the SimulatePipelineResponseTests from 10 to 5 to reduce the testing time required for parsing.

* 6.x: Fix broken backport of #31578 by adjusting constructor (#31587) ingest: Add ignore_missing property to foreach filter (#22147) (#31578) Add package pre-install check for java binary (#31343) Docs: Clarify sensitive fields watcher encryption (#31551) Watcher: Remove never executed code (#31135) Improve test times for tests using `RandomObjects::addFields` (#31556) Revert "Remove RestGetAllAliasesAction (#31308)" REST high-level client: add simulate pipeline API (#31158) Get Mapping API to honour allow_no_indices and ignore_unavailable (#31507) Fix Mockito trying to mock IOException that isn't thrown by method (#31433) (#31527) [Test] Add full cluster restart test for Rollup (#31533) Enhance thread context uniqueness assertion fix writeIndex evaluation for aliases (#31562) Add x-opaque-id to search slow logs (#31539) Watcher: Fix put watch action (#31524) [DOCS] Significantly improve SQL docs turn GetFieldMappingsResponse to ToXContentObject (#31544) TEST: Unmute testHistoryUUIDIsGenerated Ingest Attachment: Upgrade Tika to 1.18 (#31252) TEST: Correct the assertion arguments order (#31540)

* master: ingest: Add ignore_missing property to foreach filter (#22147) (#31578) Fix a formatting issue in the docvalue_fields documentation. (#31563) reduce log level at gradle configuration time [TEST] Close additional clients created while running yaml tests (#31575) Docs: Clarify sensitive fields watcher encryption (#31551) Watcher: Remove never executed code (#31135) Add support for switching distribution for all integration tests (#30874) Improve robustness of geo shape parser for malformed shapes (#31449) QA: Create xpack yaml features (#31403) Improve test times for tests using `RandomObjects::addFields` (#31556) [Test] Add full cluster restart test for Rollup (#31533) Enhance thread context uniqueness assertion [DOCS] Fix heading format errors (#31483) fix writeIndex evaluation for aliases (#31562) Add x-opaque-id to search slow logs (#31539) Watcher: Fix put watch action (#31524) Add package pre-install check for java binary (#31343) Reduce number of raw types warnings (#31523) Migrate scripted metric aggregation scripts to ScriptContext design (#30111) turn GetFieldMappingsResponse to ToXContentObject (#31544) Close xcontent parsers (partial) (#31513) Ingest Attachment: Upgrade Tika to 1.18 (#31252) TEST: Correct the assertion arguments order (#31540)

* elastic/master: (57 commits) HLRest: Fix test for explain API [TEST] Fix RemoteClusterConnectionTests Add Create Snapshot to High-Level Rest Client (elastic#31215) Remove legacy MetaDataStateFormat (elastic#31603) Add explain API to high-level REST client (elastic#31387) Preserve thread context when connecting to remote cluster (elastic#31574) Unify headers for full text queries Remove redundant 'minimum_should_match' JDBC driver prepared statement set* methods (elastic#31494) [TEST] call yaml client close method from test suite (elastic#31591) ingest: Add ignore_missing property to foreach filter (elastic#22147) (elastic#31578) Fix a formatting issue in the docvalue_fields documentation. (elastic#31563) reduce log level at gradle configuration time [TEST] Close additional clients created while running yaml tests (elastic#31575) Docs: Clarify sensitive fields watcher encryption (elastic#31551) Watcher: Remove never executed code (elastic#31135) Add support for switching distribution for all integration tests (elastic#30874) Improve robustness of geo shape parser for malformed shapes (elastic#31449) QA: Create xpack yaml features (elastic#31403) Improve test times for tests using `RandomObjects::addFields` (elastic#31556) ...

improve Randomobjects::addfield logic

c2b4f08

-- reduce number of documents generated for SimulatePipelineResponse

sohaibiftikhar commented Jun 25, 2018

View reviewed changes

javanna reviewed Jun 25, 2018

View reviewed changes

colings86 added >test Issues or PRs that are addressing/adding tests :Delivery/Build Build or test infrastructure labels Jun 25, 2018

simplified addFields

81c1645

javanna approved these changes Jun 26, 2018

View reviewed changes

javanna added v7.0.0 v6.4.0 labels Jun 26, 2018

javanna merged commit ca4c857 into elastic:master Jun 26, 2018

sohaibiftikhar deleted the tests_fix branch June 26, 2018 10:40

javanna added the backport pending label Jun 26, 2018

javanna removed the backport pending label Jun 26, 2018

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve test times for tests using `RandomObjects::addFields` #31556

Improve test times for tests using `RandomObjects::addFields` #31556

sohaibiftikhar commented Jun 25, 2018 •

edited

Loading

javanna commented Jun 25, 2018

sohaibiftikhar Jun 25, 2018

javanna Jun 25, 2018

javanna left a comment

javanna Jun 25, 2018

sohaibiftikhar Jun 25, 2018

javanna Jun 25, 2018

sohaibiftikhar Jun 25, 2018

javanna Jun 25, 2018

sohaibiftikhar Jun 25, 2018 •

edited

Loading

javanna Jun 25, 2018

sohaibiftikhar Jun 25, 2018

sohaibiftikhar Jun 25, 2018

elasticmachine commented Jun 25, 2018

javanna left a comment

Improve test times for tests using RandomObjects::addFields #31556

Improve test times for tests using RandomObjects::addFields #31556

Conversation

sohaibiftikhar commented Jun 25, 2018 • edited Loading

javanna commented Jun 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javanna left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sohaibiftikhar Jun 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Jun 25, 2018

javanna left a comment

Choose a reason for hiding this comment

Improve test times for tests using `RandomObjects::addFields` #31556

Improve test times for tests using `RandomObjects::addFields` #31556

sohaibiftikhar commented Jun 25, 2018 •

edited

Loading

sohaibiftikhar Jun 25, 2018 •

edited

Loading