Skip to content

[FLINK-13592][e2e] Fix hardcoded flink version in tpch end-to-end test. #9368

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.

Sign up for GitHub

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

Closed
KurtYoung wants to merge 4 commits into from
Closed

[FLINK-13592][e2e] Fix hardcoded flink version in tpch end-to-end test. #9368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5963462
[FLINK-13592][e2e] Fix hardcoded flink version in tpch end-to-end test.
KurtYoung Aug 6, 2019
01cb27e
strip off the tpch test jar version
KurtYoung Aug 6, 2019
b05f288
revert parallelism change
KurtYoung Aug 6, 2019
4e442c2
[hotfix] Change the parallelism of tpch end-to-end test to 2.
KurtYoung Aug 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions flink-end-to-end-tests/flink-tpch-test/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,22 @@ under the License.
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-shade-plugin</artifactId>
<executions>
<execution>
<id>TpchTestProgram</id>
<phase>package</phase>
<goals>
<goal>shade</goal>
</goals>
<configuration>
<finalName>TpchTestProgram</finalName>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>

Expand Down
5 changes: 3 additions & 2 deletions flink-end-to-end-tests/test-scripts/test_tpch.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ echo "Generating test data..."

TARGET_DIR="$END_TO_END_DIR/flink-tpch-test/target"
TPCH_DATA_DIR="$END_TO_END_DIR/test-scripts/test-data/tpch"
java -cp "$TARGET_DIR/flink-tpch-test-1.10-SNAPSHOT.jar:$TARGET_DIR/lib/*" org.apache.flink.table.tpch.TpchDataGenerator "$SCALE" "$TARGET_DIR"
java -cp "$TARGET_DIR/TpchTestProgram.jar:$TARGET_DIR/lib/*" org.apache.flink.table.tpch.TpchDataGenerator "$SCALE" "$TARGET_DIR"

################################################################################
# Prepare Flink
Expand Down Expand Up @@ -72,6 +72,7 @@ execution:
planner: blink
type: batch
result-mode: table
parallelism: 2
Copy link
Contributor

@dawidwys dawidwys Aug 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this change? It's unrelated to the subject of the commit.

Copy link
Contributor Author

@KurtYoung KurtYoung Aug 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a supplement to this issue: https://issues.apache.org/jira/browse/FLINK-13441. I think this is also a good place to cover such scenarios, and don't need to open a jira for it.

Copy link
Contributor

@dawidwys dawidwys Aug 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this related to the linked issue? Honestly I still don't get what is the purpose of this change.

I disagree this is a good place to add such cases. It's impossible to link this change to the reason why it was introduced. I'm not saying we need a new JIRA issue, but it should at least be in a separate hotfix commit with at least brief explanation what it does.

Copy link
Contributor Author

@KurtYoung KurtYoung Aug 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I explained this change in the commit message if you missed that:

Also changed test parallelism to 2 to cover the situation that parallelism is higher than the slot number, since the testing cluster only have one task manager and contains only one slot

I agree this can be isolated to another commit.

Copy link
Contributor

@dawidwys dawidwys Aug 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right I missed it, sorry for that. Now I get it. I think this would be a perfect description for the extracted commit (and easier to find)

Copy link
Contributor Author

@KurtYoung KurtYoung Aug 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I already did the exactly thing in my last commit.

EOF

if [[ -e "$MODIFIED_QUERY_DIR/q$i.sql" ]]
Expand All @@ -87,5 +88,5 @@ EOF

wait_job_terminal_state "$JOB_ID" "FINISHED"

java -cp "$TARGET_DIR/flink-tpch-test-1.10-SNAPSHOT.jar" org.apache.flink.table.tpch.TpchResultComparator "$EXPECTED_DIR/q$i.csv" "$RESULT_DIR/q$i.csv"
java -cp "$TARGET_DIR/TpchTestProgram.jar" org.apache.flink.table.tpch.TpchResultComparator "$EXPECTED_DIR/q$i.csv" "$RESULT_DIR/q$i.csv"
done