Skip to content

[test](docker) improve the prefix path of cloud mode docker env#65001

Merged
hello-stephen merged 2 commits into
apache:masterfrom
pingchunzhang:fix-docker-case
Jul 1, 2026
Merged

[test](docker) improve the prefix path of cloud mode docker env#65001
hello-stephen merged 2 commits into
apache:masterfrom
pingchunzhang:fix-docker-case

Conversation

@pingchunzhang

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason
  • Behavior changed:

    • No.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@pingchunzhang

Copy link
Copy Markdown
Contributor Author

/review

@pingchunzhang

Copy link
Copy Markdown
Contributor Author

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor

/review

@hello-stephen hello-stephen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@github-actions github-actions Bot added approved Indicates a PR has been approved by one committer. reviewed labels Jul 1, 2026
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@deardeng deardeng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Found one correctness gap in the docker-compose cloud prefix change.

Critical checkpoint conclusions: the task is narrow and focused, but the implementation misses a functionally parallel instance-creation path. No concurrency, storage-format, FE/BE protocol, transaction, persistence, or lifecycle issue was introduced by this Python-only helper change. No new Doris config item is added. Python syntax compilation passed for the compose helper files; full docker/regression execution was not run in this review runner. Test coverage is weak for the changed behavior because the PR changes only cluster.py and does not add an external-MS/custom-prefix regression case.

Subagent conclusions: optimizer-rewrite reported no optimizer, query-routing, or parallel execution issue. tests-session-config proposed TSC-1, which was verified and accepted as the inline comment. The separate cloud.ini.example/cloud-config ambiguity was dismissed with code evidence because the changed behavior can be driven through --env, while the accepted issue is the external-MS auto-create path. Final convergence round 1 ended with both live subagents reporting NO_NEW_VALUABLE_FINDINGS for the same ledger/comment set.

User focus: no additional user-provided review focus points.

time.strftime("%Y%m%d_%H%M%S"),
uuid.uuid4().hex[:8],
)
base_prefix = envs.get('DORIS_CLOUD_PREFIX', '').strip().strip('/')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This only applies the base-prefix rewrite on MS containers, but the --external-ms flow creates the instance from FE-1 instead. In that path FE.docker_env() sets AUTO_CREATE_INSTANCE=1 and carries the user env through unchanged, then init_fe.sh calls create_doris_instance, which sends ${DORIS_CLOUD_PREFIX} directly as the storage prefix. So --env DORIS_CLOUD_PREFIX=base works for a normal cloud cluster as base/doris_docker_env_..., but an external-MS cluster stores everything under just base, causing different external-MS clusters that share the same bucket/base to collide. Please share this prefix composition with the FE auto-create path, or otherwise ensure the generated per-cluster suffix is applied before FE-1 creates the instance.

@hello-stephen hello-stephen merged commit ed823b6 into apache:master Jul 1, 2026
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. dev/4.0.x dev/4.0.x-conflict dev/4.1.x reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants