Skip to content

Conversation

@XComp
Copy link
Contributor

@XComp XComp commented Apr 4, 2022

What is the purpose of the change

Having a default RpcSystem in TestingDispatcher.Builder
caused threads being spawned without cleanup. Instead, we
should rely on the RpcSystem instance provided by the test.

Brief change log

  • Removed the default instance from TestingDispatcher.Builder and made it a required parameter through the build method

Verifying this change

  • Running a test manually in a loop and checking that there are no threads left hanging, e.g. in DispatcherCleanupResourcesTest:
    @Test
    public void foo() throws Exception {
        for (int i = 0; i < 100; i++) {
            setup();
            testDuplicateJobSubmissionDoesNotDeleteJobMetaData();
            teardown();
        }

        System.out.println("Stop the debugger here");
    }

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, Kubernetes/Yarn, 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? not applicable

@flinkbot
Copy link
Collaborator

flinkbot commented Apr 4, 2022

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

Copy link
Contributor

@zentol zentol left a comment

Choose a reason for hiding this comment

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

+1

@XComp
Copy link
Contributor Author

XComp commented Apr 6, 2022

I'm investigating the OOM in the integration tests of the flink-tests module (ci run). In the mean time, I'm gonna start another ci run

@XComp
Copy link
Contributor Author

XComp commented Apr 6, 2022

@flinkbot run azure

@XComp
Copy link
Contributor Author

XComp commented Apr 6, 2022

Force push without additional changes to trigger the CI since the run azure didn't seem to have worked...

@XComp XComp force-pushed the FLINK-27050 branch 2 times, most recently from a83d4e1 to 629dfa3 Compare April 7, 2022 07:30
@XComp
Copy link
Contributor Author

XComp commented Apr 7, 2022

Again force-pushed without any additional changes to trigger ci once more...

@zentol
Copy link
Contributor

zentol commented Apr 7, 2022

@flinkbot run azure

@XComp
Copy link
Contributor Author

XComp commented Apr 7, 2022

Rebased to most-recent master

Having a default RpcSystem in TestingDispatcher.Builder
caused threads being spawned without cleanup. Instead, we
should rely on the RpcSystem instance provided by the test.
@XComp
Copy link
Contributor Author

XComp commented Apr 12, 2022

Force-pushed once more after the branch idle'd for 5 days to prepare the merge for tomorrow morning...

@XComp
Copy link
Contributor Author

XComp commented Apr 13, 2022

Error caused by FLINK-26621 which is unrelated to this change.

@XComp XComp merged commit ccf4d24 into apache:master Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants