Rework on serialization and deserialization by Yury-Fridlyand · Pull Request #252 · Bit-Quill/opensearch-project-sql

Yury-Fridlyand · 2023-03-21T22:41:37Z

Description

Rework PhysicalPlan tree serialization and deserialization. Make it scalable and extendable.
If a new plan should be serialized (a new feature is becoming supported by pagination) - it should properly overload SerializablePlan::writeExternal only.
The plan tree will be recovered in deserialization the same way it was before serialization.
New mechanism can also skip plans from serialization - for example, it skips ResourceMonitorPlan.

Issues Resolved

opensearch-project#1483

Check List

New functionality includes testing.
- All tests pass, including unit test, integration test and doctest
New functionality has been documented.
- New functionality has javadoc added
- New functionality has user manual doc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

core/src/main/java/org/opensearch/sql/planner/physical/PaginateOperator.java

core/src/main/java/org/opensearch/sql/planner/physical/ProjectOperator.java

...earch/src/main/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchPagedIndexScan.java

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Step 2. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

core/src/main/java/org/opensearch/sql/planner/SerializablePlan.java

core/src/main/java/org/opensearch/sql/executor/pagination/PaginatedPlanCache.java

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

MaxKsyunz · 2023-04-05T01:42:04Z

core/src/main/java/org/opensearch/sql/executor/pagination/PaginatedPlanCache.java

+      return new Cursor(CURSOR_PREFIX
+          + serialize(((SerializablePlan) plan).getPlanForSerialization()));
+      // ClassCastException thrown when a plan in the tree doesn't implement SerializablePlan
+    } catch (ClassCastException | NoCursorException e) {


When can ClassCastException be thrown?

We are always trying to convert a plan tree to a cursor in

opensearch-project-sql/opensearch/src/main/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngine.java

Line 55 in 10ba1d6

plan.getTotalHits(), paginatedPlanCache.convertToCursor(plan));

If tree contains a plan which doesn't implement SerializablePlan, there would be a ClassCastException somewhere, e.g. in

opensearch-project-sql/opensearch/src/main/java/org/opensearch/sql/opensearch/executor/protector/ResourceMonitorPlan.java

Lines 93 to 96 in 10ba1d6

@Override

public SerializablePlan getPlanForSerialization() {

return (SerializablePlan) delegate;

}

I don't serialize PaginateOperator (to reduce cursor size), so I can't check plan instanceof PaginateOperator here as it was before.

opensearch-project-sql/core/src/main/java/org/opensearch/sql/planner/physical/PaginateOperator.java

Lines 69 to 73 in 0ff3390

/** No need to serialize a PaginateOperator, it actually does nothing - it is a wrapper. */

@Override

public SerializablePlan getPlanForSerialization() {

return (SerializablePlan) input;

}

That means I have to do another protection here. The incoming tree could be ProjectOperator -> ResourceMonitorPlan -> PrometheusIndexScan or ProjectOperator -> ValuesOperator.

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Yury-Fridlyand · 2023-04-05T21:32:39Z

Superseded by opensearch-project#1498

Yury-Fridlyand mentioned this pull request Mar 24, 2023

Support pagination in V2 engine, phase 1 #226

Merged

18 tasks

Yury-Fridlyand commented Mar 24, 2023

View reviewed changes

core/src/main/java/org/opensearch/sql/planner/physical/PaginateOperator.java Outdated Show resolved Hide resolved

core/src/main/java/org/opensearch/sql/planner/physical/ProjectOperator.java Outdated Show resolved Hide resolved

Yury-Fridlyand commented Mar 24, 2023

View reviewed changes

...earch/src/main/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchPagedIndexScan.java Outdated Show resolved Hide resolved

Yury-Fridlyand mentioned this pull request Mar 28, 2023

Support pagination in V2 engine, phase 1 (#226) opensearch-project/sql#1483

Closed

6 tasks

Step 1.

ef44600

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Step 2. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Yury-Fridlyand changed the base branch from dev-pagination-v2_revC to integ-pagination-p1 March 29, 2023 20:33

Yury-Fridlyand force-pushed the dev-pagination-v2_revD branch from 7a9d285 to ef44600 Compare March 29, 2023 20:33

Yury-Fridlyand added 5 commits March 29, 2023 21:13

Minor cleanup.

93a82ad

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Optimize serialization of ProjectOperator.

ecc1c9a

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Minor cleaning and commenting.

430323c

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Move ExpressionSerializer back to :opensearch.

8350d9c

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Add tests.

67ce92d

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

This comment was marked as spam.

Sign in to view

MaxKsyunz reviewed Mar 31, 2023

View reviewed changes

Yury-Fridlyand marked this pull request as ready for review March 31, 2023 23:51

Yury-Fridlyand requested review from GabeFernandez310, GumpacG, acarbonetto, forestmvey and matthewryanwells March 31, 2023 23:51

Address PR feedback.

490af96

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

MaxKsyunz approved these changes Apr 4, 2023

View reviewed changes

Complete rework on serialization and deserialization.

10ba1d6

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

MaxKsyunz reviewed Apr 5, 2023

View reviewed changes

Minor fix for serialization.

0ff3390

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

This was referenced Apr 5, 2023

Support pagination in V2 engine, phase 1 opensearch-project/sql#1497

Merged

Rework on serialization and deserialization in pagination opensearch-project/sql#1498

Merged

Yury-Fridlyand closed this Apr 5, 2023

Yury-Fridlyand deleted the dev-pagination-v2_revD branch June 24, 2023 00:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework on serialization and deserialization#252

Rework on serialization and deserialization#252
Yury-Fridlyand wants to merge 9 commits intointeg-pagination-p1from
dev-pagination-v2_revD

Yury-Fridlyand commented Mar 21, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as spam.

Uh oh!

Uh oh!

Uh oh!

MaxKsyunz Apr 5, 2023

Uh oh!

Yury-Fridlyand Apr 5, 2023

Uh oh!

Yury-Fridlyand Apr 5, 2023

Uh oh!

Yury-Fridlyand commented Apr 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	@Override
	public SerializablePlan getPlanForSerialization() {
	return (SerializablePlan) delegate;
	}

	/** No need to serialize a PaginateOperator, it actually does nothing - it is a wrapper. */
	@Override
	public SerializablePlan getPlanForSerialization() {
	return (SerializablePlan) input;
	}

Conversation

Yury-Fridlyand commented Mar 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues Resolved

Check List

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as spam.

Uh oh!

Uh oh!

Uh oh!

MaxKsyunz Apr 5, 2023

Choose a reason for hiding this comment

Uh oh!

Yury-Fridlyand Apr 5, 2023

Choose a reason for hiding this comment

Uh oh!

Yury-Fridlyand Apr 5, 2023

Choose a reason for hiding this comment

Uh oh!

Yury-Fridlyand commented Apr 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Yury-Fridlyand commented Mar 21, 2023 •

edited

Loading