Skip to content

[Build] Skip etcd go package compilation by default#520

Merged
ykwd merged 3 commits intokvcache-ai:mainfrom
ykwd:hotfix/go-compile
Jun 20, 2025
Merged

[Build] Skip etcd go package compilation by default#520
ykwd merged 3 commits intokvcache-ai:mainfrom
ykwd:hotfix/go-compile

Conversation

@ykwd
Copy link
Copy Markdown
Collaborator

@ykwd ykwd commented Jun 19, 2025

As suggested in the comments of #501 , we'd better let users build mooncake-store without compiling the go etcd wrapper.

Content of this PR:

  • By default, the STORE_USE_ETCD compile option is off. And the compiling of store no longer depends on the go etcd wrapper.
  • If users want to use the high availability feature in store, then set the STORE_USE_ETCD to on.
  • In the CI and Release, this option is set as on

@ykwd ykwd marked this pull request as ready for review June 19, 2025 07:47
@staryxchen
Copy link
Copy Markdown
Collaborator

I believe using a compilation option named ​USE_STORE_HA​ would be more appropriate. Here's why:

​ETCD is a core dependency​ for the current High Availability (HA) feature implementation:

  • When HA is ​not enabled, ETCD is naturally not required

  • When HA ​is enabled, ETCD ​must​ be used - there's no way to enable HA without it now

Therefore, ​if users want to enable HA features, they should be explicitly required to include ETCD wrapper at compilation time​ without the ability to circumvent this dependency. This ensures architectural consistency and prevents runtime failures.

In summary, I think an option to control the whole HA feature is better.

@ykwd
Copy link
Copy Markdown
Collaborator Author

ykwd commented Jun 19, 2025

I believe using a compilation option named ​USE_STORE_HA​ would be more appropriate. Here's why:

​ETCD is a core dependency​ for the current High Availability (HA) feature implementation:

  • When HA is ​not enabled, ETCD is naturally not required
  • When HA ​is enabled, ETCD ​must​ be used - there's no way to enable HA without it now

Therefore, ​if users want to enable HA features, they should be explicitly required to include ETCD wrapper at compilation time​ without the ability to circumvent this dependency. This ensures architectural consistency and prevents runtime failures.

In summary, I think an option to control the whole HA feature is better.

Good suggestion. Actually, I am also hesitating about using USE_STORE_HA. As you mentioned, this will be easier to understand and more straightforward to use. On the other hand, there are drawbacks, too:

  • Currently, ETCD is only used for leader election and leader discovery. Some other features, such as tolerance of client failures, do not rely on ETCD. So, in the future, we may be able to enable these features without etcd.
  • We may have alternative choices for the leader election and leader discovery in the future, such as raft, or redis. So users can select which to use.

Copy link
Copy Markdown
Collaborator

@stmatengss stmatengss left a comment

Choose a reason for hiding this comment

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

Etcd installation is painful for users who don't need it. LGTM

@doujiang24
Copy link
Copy Markdown
Collaborator

Hi @ykwd , I'm a bit confused about the difference between USE_ETCD and STORE_USE_ETCD.
What does this means? -DUSE_ETCD=ON -DSTORE_USE_ETCD=OFF.

@ykwd
Copy link
Copy Markdown
Collaborator Author

ykwd commented Jun 20, 2025

Hi @ykwd , I'm a bit confused about the difference between USE_ETCD and STORE_USE_ETCD. What does this means? -DUSE_ETCD=ON -DSTORE_USE_ETCD=OFF.

It is indeed a little confusing. Let me explain:

  • USE_ETCD means transfer engine uses etcd as its meta server. Other choices for the meta server include redis and HTTP server.
  • STORE_USE_ETCD means mooncake-store uses etcd for master server's failover, such as leader election and leader discovery. Currently, the failover feature only supports using etcd as its backend.
  • Setting USE_ETCD to ON and USE_ETCD_LEGACY to FALSE or setting STORE_USE_ETCD to ON will compile the go interface for accessing etcd, which depends on go.
  • Setting USE_ETCD_LEGACY to ON will compile the c++ interface for accessing etcd from transfer engine. But as the maintenance of c++ interface is less active compared to the go interface, this is named as legacy. Currently store does not support this option.

@xiaguan
Copy link
Copy Markdown
Collaborator

xiaguan commented Jun 20, 2025

You can take a look at this PR as a reference for adding a CI test(#473), so we can make sure it builds successfully without needing Go installed

@ykwd
Copy link
Copy Markdown
Collaborator Author

ykwd commented Jun 20, 2025

I updated the readme doc to make it less confusing

@ykwd
Copy link
Copy Markdown
Collaborator Author

ykwd commented Jun 20, 2025

You can take a look at this PR as a reference for adding a CI test(#473), so we can make sure it builds successfully without needing Go installed

Good idea. Given that the build-without-store task in the CI test(#473) contains a large amount of code, we may want to use a matrix strategy to reuse it across different compile options.

@ykwd ykwd merged commit fd92bc7 into kvcache-ai:main Jun 20, 2025
10 checks passed
@ykwd ykwd deleted the hotfix/go-compile branch July 10, 2025 06:36
wanyue-wy pushed a commit to wanyue-wy/Mooncake that referenced this pull request Dec 14, 2025
)

By default, the STORE_USE_ETCD compile option is OFF. In this case, the compiling of store does not depend on the go etcd wrapper.
If users want to use the high availability feature in store, then set the STORE_USE_ETCD to ON.
In the CI and Release, this option is set to ON.
Update the readme to explain this compile option.
JasonZhang517 pushed a commit to JasonZhang517/Mooncake that referenced this pull request Feb 9, 2026
)

By default, the STORE_USE_ETCD compile option is OFF. In this case, the compiling of store does not depend on the go etcd wrapper.
If users want to use the high availability feature in store, then set the STORE_USE_ETCD to ON.
In the CI and Release, this option is set to ON.
Update the readme to explain this compile option.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants