Skip to content

Conversation

snuyanzin
Copy link
Contributor

@snuyanzin snuyanzin commented Sep 26, 2025

What is the purpose of the change

The PR make the test more stable by making prefix unique per test.

The problem with the test that all use same prefix and at some point it counts items in profiling list by prefix which is same for all in this test. The problem could be reproduced while running tests in parallel (usually happens while ./mvnw clean install)

Brief change log

Make prefixes unique per test

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

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 Sep 26, 2025

CI report:

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

@snuyanzin
Copy link
Contributor Author

@flinkbot run azure

@snuyanzin snuyanzin requested a review from Copilot September 27, 2025 08:27
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves the stability of ProfilingServiceTest.testRollingDeletion by ensuring unique prefixes per test method. The issue was that all tests used the same resource ID prefix, causing race conditions when tests run in parallel during ./mvnw clean install.

  • Modified test methods to accept TestInfo parameter for unique identification
  • Updated resource ID generation to include test method name as suffix
  • Enhanced rolling deletion verification to use test-specific resource IDs

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@snuyanzin snuyanzin requested a review from Myasuka September 27, 2025 08:27
@snuyanzin
Copy link
Contributor Author

@yuchen-ecnu , @Myasuka may I ask you to take a look since you were working on this in past

@github-actions github-actions bot added the community-reviewed PR has been reviewed by the community. label Sep 27, 2025
@snuyanzin snuyanzin removed the community-reviewed PR has been reviewed by the community. label Sep 28, 2025
@github-actions github-actions bot added the community-reviewed PR has been reviewed by the community. label Sep 28, 2025
@snuyanzin snuyanzin removed the community-reviewed PR has been reviewed by the community. label Sep 29, 2025
@github-actions github-actions bot added the community-reviewed PR has been reviewed by the community. label Sep 29, 2025
@snuyanzin snuyanzin removed the community-reviewed PR has been reviewed by the community. label Sep 30, 2025
@github-actions github-actions bot added the community-reviewed PR has been reviewed by the community. label Sep 30, 2025
@snuyanzin snuyanzin removed the community-reviewed PR has been reviewed by the community. label Sep 30, 2025
@github-actions github-actions bot added the community-reviewed PR has been reviewed by the community. label Oct 1, 2025
profilingService.requestProfiling(RESOURCE_ID, duration, mode).get();
profilingService
.requestProfiling(
RESOURCE_ID + "_" + testInfo.getTestMethod().get().getName(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just use test name as part of prefix to make it unique per test and to avoid tests impacting each other while concurrent execution

@snuyanzin snuyanzin merged commit 6d738f4 into apache:master Oct 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-reviewed PR has been reviewed by the community.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants