[FLINK-15214] Introduce multiple submission e2e test #10666

KarmaGYZ · 2019-12-23T11:31:08Z

What is the purpose of the change

Introduce multiple submission e2e test for Flink's Mesos integration.

Brief change log

43b1425..2885bab: Hotfixes, refactors and code clean-ups.
ab3c8ac: Introduce multiple submission e2e test
cc596f9: Enable it in Travis and nightly script

Verifying this change

Run multiple submission e2e test
./run-single-test.sh test-scripts/test_mesos_multi_submission.sh

Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): no
The public API, i.e., is any changed class annotated with @Public(Evolving): no
The serializers: no
The runtime per-record code paths (performance sensitive): no
Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: no
The S3 file system connector: no

Documentation

Does this pull request introduce a new feature? no
If yes, how is the feature documented? no

flinkbot · 2019-12-23T11:33:36Z

Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.

Automated Checks

Last check on commit cc596f9 (Mon Dec 23 11:33:36 UTC 2019)

Warnings:

No documentation files were touched! Remember to keep the Flink docs up to date!
This pull request references an unassigned Jira ticket. According to the code contribution guide, tickets need to be assigned before starting with the implementation work.

_{Mention the bot in a comment to re-run the automated checks.}

Review Progress

❓ 1. The [description] looks good.
❓ 2. There is [consensus] that the contribution should go into to Flink.
❓ 3. Needs [attention] from.
❓ 4. The change fits into the overall [architecture].
❓ 5. Overall code [quality] is good.

Please see the Pull Request Review Guide for a full explanation of the review process.

The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot approve description to approve one or more aspects (aspects: description, consensus, architecture and quality)
@flinkbot approve all to approve all aspects
@flinkbot approve-until architecture to approve everything until architecture
@flinkbot attention @username1 [@username2 ..] to require somebody's attention
@flinkbot disapprove architecture to remove an approval you gave earlier

flinkbot · 2019-12-23T11:54:04Z

CI report:

cc596f9 Travis: SUCCESS Azure: SUCCESS
4f0363b Travis: SUCCESS Azure: SUCCESS
c2cd715 Travis: FAILURE Azure: SUCCESS
9113fc4 Travis: SUCCESS Azure: SUCCESS
5a5c93d Travis: SUCCESS Azure: FAILURE

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run travis re-run the last Travis build
@flinkbot run azure re-run the last Azure build

flink-end-to-end-tests/test-scripts/test_mesos_multi_submission.sh

flink-end-to-end-tests/test-scripts/common.sh

flink-end-to-end-tests/test-scripts/test_mesos_multi_submission.sh

flink-end-to-end-tests/run-nightly-tests.sh

flink-end-to-end-tests/test-scripts/common.sh

KarmaGYZ · 2020-01-06T02:35:28Z

Thanks for the review @GJL . All the comments addressed.

KarmaGYZ · 2020-01-06T03:27:52Z

Travis gives green light to relevant test. https://travis-ci.org/KarmaGYZ/flink/builds/633110748

KarmaGYZ · 2020-01-06T06:29:54Z

@flinkbot run travis

GJL

LGTM. I only had some minor comments.

flink-end-to-end-tests/test-scripts/test_mesos_multiple_submissions.sh

flink-end-to-end-tests/test-scripts/docker-mesos-cluster/docker-compose.yml

flink-end-to-end-tests/test-scripts/common.sh

GJL · 2020-01-06T12:45:59Z

tools/travis/splits/split_container.sh

@@ -51,6 +51,7 @@ run_test "Run kubernetes test" "$END_TO_END_DIR/test-scripts/test_kubernetes_emb
 run_test "Run kubernetes session test" "$END_TO_END_DIR/test-scripts/test_kubernetes_session.sh"
 if [[ "${HADOOP_INTEGRATION}" = "with-hadoop" ]]; then
    run_test "Run Mesos WordCount test" "$END_TO_END_DIR/test-scripts/test_mesos_wordcount.sh"


After adding the new multiple submissions test, do we still need the word count test?

I think we still need a "openbox" test case for deploying Flink on Mesos cluster. Anyway, it should be discussed in another ticket. WDYT?

Can you define what you mean by "openbox"?

I think each test case better not have multiple purposes so that we could find out the root cause quickly. I treat the WordCount case like a test for basic process of running Flink job on Mesos, including starting appmaster, running a simple job and verifying the correctness of result. I prefer to keep it in release 1.10, but I'm ok to remove it when the time overhead of E2E tests become severe.

I agree that a separate test makes it easier to locate the root cause. However, due to being expensive, E2E tests should not be used for exhaustively testing all cases. Currently, running all E2E tests take more than 24 CPU hours on Travis. e2e - container - hadoop 2.8 takes ~43 minutes on Travis, which is not great but acceptable. Imo, if we decide to add more Mesos tests, we should start to consolidate soon.

cc: @tillrohrmann

I agree with your conclusion that ideally we try to separate test cases but in this case it might acceptable to consolidate them in order to save build time.

Fine. So, in which release version we need to remove the WordCount case? 1.10 or 1.11? I prefer to remove it in next release version, when two other Mesos test cases add. I don't have a strong opinion though.

I am ok to consolidate them with the next test that we add.

…ework

…ence_in_logs function

KarmaGYZ · 2020-01-07T03:10:42Z

PR updated. Travis link https://travis-ci.org/KarmaGYZ/flink/builds/633584594

GJL · 2020-01-07T14:36:28Z

Merging.

This closes #10666.

rmetzger added the review=description? label Dec 23, 2019

rmetzger added the component=Tests label Dec 23, 2019

KarmaGYZ force-pushed the FLINK-15214 branch from cc596f9 to 4f0363b Compare January 2, 2020 09:50

GJL reviewed Jan 2, 2020

View reviewed changes

KarmaGYZ force-pushed the FLINK-15214 branch from 4f0363b to c2cd715 Compare January 3, 2020 04:04

GJL reviewed Jan 3, 2020

View reviewed changes

flink-end-to-end-tests/test-scripts/common.sh Outdated Show resolved Hide resolved

KarmaGYZ force-pushed the FLINK-15214 branch from c2cd715 to 9113fc4 Compare January 6, 2020 02:26

GJL approved these changes Jan 6, 2020

View reviewed changes

KarmaGYZ added 5 commits January 7, 2020 10:06

[hotfix][e2e] Enable waiting for job tesrminal state in multiple Fram…

1eaa20c

…ework

[hotfix][e2e] Normalize the way to get logs name in wait_num_of_occur…

9200ba7

…ence_in_logs function

[hotfix] Introduce MESOS_AGENT_CPU variable in Mesos e2e test infra

1038d0d

[FLINK-15214] Introduce multiple submission e2e test

0654378

[FLINK-15214] Enable multiple submission e2e test in Travis

5a5c93d

KarmaGYZ force-pushed the FLINK-15214 branch from 9113fc4 to 5a5c93d Compare January 7, 2020 02:13

GJL closed this in ec19b7a Jan 7, 2020

GJL pushed a commit that referenced this pull request Jan 7, 2020

[FLINK-15214][e2e] Introduce multiple submissions e2e test for mesos

1938498

This closes #10666.

KarmaGYZ deleted the FLINK-15214 branch January 11, 2021 02:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FLINK-15214] Introduce multiple submission e2e test #10666

[FLINK-15214] Introduce multiple submission e2e test #10666

KarmaGYZ commented Dec 23, 2019 •

edited

flinkbot commented Dec 23, 2019

flinkbot commented Dec 23, 2019 •

edited

KarmaGYZ commented Jan 6, 2020

KarmaGYZ commented Jan 6, 2020

KarmaGYZ commented Jan 6, 2020

GJL left a comment

GJL Jan 6, 2020

KarmaGYZ Jan 6, 2020

GJL Jan 7, 2020

KarmaGYZ Jan 7, 2020

GJL Jan 7, 2020

tillrohrmann Jan 8, 2020

KarmaGYZ Jan 9, 2020

GJL Jan 9, 2020

KarmaGYZ commented Jan 7, 2020

GJL commented Jan 7, 2020

[FLINK-15214] Introduce multiple submission e2e test #10666

[FLINK-15214] Introduce multiple submission e2e test #10666

Conversation

KarmaGYZ commented Dec 23, 2019 • edited

What is the purpose of the change

Brief change log

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

flinkbot commented Dec 23, 2019

Automated Checks

Review Progress

flinkbot commented Dec 23, 2019 • edited

CI report:

KarmaGYZ commented Jan 6, 2020

KarmaGYZ commented Jan 6, 2020

KarmaGYZ commented Jan 6, 2020

GJL 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KarmaGYZ commented Jan 7, 2020

GJL commented Jan 7, 2020

KarmaGYZ commented Dec 23, 2019 •

edited

flinkbot commented Dec 23, 2019 •

edited