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

refactor: conditionally compile wal impls #1272

Merged
merged 17 commits into from
Oct 21, 2023
Merged

Conversation

tisonkun
Copy link
Member

@tisonkun tisonkun commented Oct 19, 2023

Rationale

Supersede #1231

After compile time with clean -

    Finished dev [unoptimized + debuginfo] target(s) in 2m 18s

Detailed Changes

Conditionally compile wal impls with feature flags.

Test Plan

Code refactor. All existing tests should pass.

Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
@jiacai2050
Copy link
Contributor

@tisonkun How big is the final binary?

Current size is 1.3G, which is a pain for distribution.

@tisonkun
Copy link
Member Author

@jiacai2050 with make release-slim it gives:

l target/release-slim/ceresdb-server 
-rwxr-xr-x  1 tison  staff    83M Oct 20 15:01 target/release-slim/ceresdb-server

@tisonkun
Copy link
Member Author

tisonkun commented Oct 20, 2023

Updated. PTAL again @jiacai2050 @ShiKaiWi

Signed-off-by: tison <wander4096@gmail.com>
@ShiKaiWi
Copy link
Member

The default feature should include all the WAL implementations, to avoid any possible confusion about data loss, config not working, etc.

@tisonkun If we turn on all the implementations in the default features, it will be better, and there is no need to update the dockerfile and the ci. 😄

Signed-off-by: tison <wander4096@gmail.com>
@tisonkun
Copy link
Member Author

Updated. PTAL again.

BTW, golang related CI failed with a new issue:

server/coordinator/scheduler/static/scheduler.go:6:2: package cmp is not in GOROOT (/opt/hostedtoolcache/go/1.20.10/x64/src/cmp)
note: imported by a module that requires go 1.21
server/coordinator/scheduler/rebalanced/scheduler.go:8:2: package maps is not in GOROOT (/opt/hostedtoolcache/go/1.20.10/x64/src/maps)
note: imported by a module that requires go 1.21
server/coordinator/scheduler/nodepicker/hash/consistent_uniform.go:38:2: package slices is not in GOROOT (/opt/hostedtoolcache/go/1.20.10/x64/src/slices)
note: imported by a module that requires go 1.21
make[1]: *** [Makefile:37: build-meta] Error 1
make[1]: Leaving directory '/home/runner/work/ceresdb/ceresdb/integration_tests'

@tisonkun
Copy link
Member Author

Perhaps we should setup go toolchain in the CI env.

@tisonkun
Copy link
Member Author

tisonkun commented Oct 21, 2023

CI passed. PTAL @jiacai2050 @ShiKaiWi

Copy link
Member

@ShiKaiWi ShiKaiWi left a comment

Choose a reason for hiding this comment

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

👍

@ShiKaiWi ShiKaiWi merged commit 31c9edf into apache:main Oct 21, 2023
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.

3 participants