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

Add chunk-dedup flag to support local cas chunk deduplication. #519

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

xwb1136021767
Copy link

@xwb1136021767 xwb1136021767 commented Aug 10, 2023

Add chunk-dedup flag to support local cas chunk deduplication. If you want to enable the local cas chunk deduplication function, start nydus-snapshotter with --chunk-dedup flag and add dedup related configurations to the nydus configuration file.

example:

"dedup": {
    "work_dir": "/var/lib/containerd-nydus"
}

@codecov
Copy link

codecov bot commented Aug 10, 2023

Codecov Report

Merging #519 (eaf7963) into main (a6f4457) will decrease coverage by 0.11%.
The diff coverage is 25.31%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #519      +/-   ##
==========================================
- Coverage   37.81%   37.70%   -0.11%     
==========================================
  Files          60       60              
  Lines        7090     7163      +73     
==========================================
+ Hits         2681     2701      +20     
- Misses       4097     4149      +52     
- Partials      312      313       +1     
Files Changed Coverage Δ
config/config.go 30.71% <0.00%> (-0.68%) ⬇️
config/daemonconfig/daemonconfig.go 0.00% <0.00%> (ø)
config/daemonconfig/fscache.go 0.00% <ø> (ø)
config/global.go 30.86% <0.00%> (-0.79%) ⬇️
pkg/daemon/daemon.go 0.00% <0.00%> (ø)
pkg/manager/daemon_adaptor.go 0.00% <0.00%> (ø)
pkg/manager/manager.go 14.88% <0.00%> (ø)
pkg/daemon/rafs.go 14.16% <44.82%> (+9.77%) ⬆️
internal/flags/flags.go 100.00% <100.00%> (ø)

config/config.go Outdated Show resolved Hide resolved
config/config.go Outdated
@@ -326,6 +327,10 @@ func ParseParameters(args *flags.Args, cfg *SnapshotterConfig) error {
daemonConfig.FsDriver = args.FsDriver
}

if args.ChunkDedup {
Copy link
Member

Choose a reason for hiding this comment

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

You can not only check if this option is true because the case of --chunk-dedup=false is omitted.

Please refer to the pull request at #391.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

config/daemonconfig/daemonconfig.go Outdated Show resolved Hide resolved
@@ -45,6 +45,10 @@ type FscacheDaemonConfig struct {
CacheConfig struct {
WorkDir string `json:"work_dir"`
} `json:"cache_config"`
DedupConfig struct {
Copy link
Member

Choose a reason for hiding this comment

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

ditto.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

config/daemonconfig/daemonconfig.go Outdated Show resolved Hide resolved
pkg/daemon/daemon.go Outdated Show resolved Hide resolved
@@ -260,6 +262,15 @@ func (d *Daemon) sharedFusedevMount(rafs *Rafs) error {
return errors.Wrap(err, "dump instance configuration")
}

// static dedup bootstrap
if d.States.ChunkDedup {
log.L.Infoln("sharedFusedevMount, cfg = ", cfg)
Copy link
Member

Choose a reason for hiding this comment

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

Please avoid printing too many logs. I suppose Debug level is more reasonable.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

pkg/daemon/rafs.go Outdated Show resolved Hide resolved
pkg/daemon/rafs.go Show resolved Hide resolved
@@ -118,6 +120,10 @@ type DeviceConfig struct {
DisableIndexedMap bool `json:"disable_indexed_map"`
} `json:"config"`
} `json:"cache"`
Dedup struct {
Enable bool `json:"enable"`
WorkDir string `json:"work_dir"`
Copy link
Member

@sctb512 sctb512 Aug 15, 2023

Choose a reason for hiding this comment

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

Is this WorkDir option redundant? Otherwise, it should be exported as well to the configuration file.

Copy link
Author

Choose a reason for hiding this comment

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

Is this WorkDir option redundant? Otherwise, it should be exported as well to the configuration file.

Nydusd needs WorkDir to specify the location of the CAS database. "Exported to the configuration file" means to dump WorkDir into the config file passed to nydusd?

Signed-off-by: xwb1136021767 <1136021767@qq.com>
@sctb512
Copy link
Member

sctb512 commented Sep 22, 2023

Need rebase. 😉

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.

None yet

2 participants