Skip to content
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.

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

Consolidate entrypoint to support broader deployment scenarios #6566

Merged
merged 1 commit into from
Mar 28, 2025

Conversation

wanpdsantos
Copy link
Contributor

What problem does this PR solve?

This PR gives better control over how we distribute which service will be loaded. With this approach, we can create containers to run only the web server and others to run the task executor. It also introduces the unique ID per task executor host, this will be important when scaling task executors horizontally, considering unique task executor ids will be required.

This new entrypoint.sh maintains the default behavior of starting the web server and task executor in the same host.

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. 💞 feature Feature request, pull request that fullfill a new feature. labels Mar 26, 2025
@wanpdsantos wanpdsantos force-pushed the consolidate-entrypoint branch from 4858414 to a1b79ce Compare March 26, 2025 09:47
@KevinHuSh KevinHuSh added the ci Continue Integration label Mar 27, 2025
@KevinHuSh
Copy link
Collaborator

Appreciations!
Fail to run:
image

@KevinHuSh KevinHuSh removed the ci Continue Integration label Mar 27, 2025
…trypoint with flexible flags

- Merged the two original scripts (webserver & taskexecutor) into a unified `entrypoint.sh`.
- Centralized environment variable replacement in `service_conf.yaml`.
- Added CLI flags to enable/disable web server and task executor:
  - `--disable-webserver` to disable start nginx + `ragflow_server`.
  - `--disable-taskexecutor` to disable the task executor.
  - `--host-id` to control unique host ID.
- Preserved existing functionality (`LD_LIBRARY_PATH`, `jemalloc`, etc.) and ensured the container remains alive by waiting on background processes.
@wanpdsantos wanpdsantos force-pushed the consolidate-entrypoint branch from a1b79ce to 143c1ee Compare March 28, 2025 01:32
@wanpdsantos
Copy link
Contributor Author

@KevinHuSh , updated the PR, now it should work as expected.

@KevinHuSh KevinHuSh added the ci Continue Integration label Mar 28, 2025
@KevinHuSh KevinHuSh requested a review from yuzhichang March 28, 2025 04:14
Copy link
Member

@yuzhichang yuzhichang left a comment

Choose a reason for hiding this comment

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

LGTM

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 28, 2025
@KevinHuSh KevinHuSh merged commit 2632493 into infiniflow:main Mar 28, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Continue Integration 💞 feature Feature request, pull request that fullfill a new feature. lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants