[skip changelog] pinmfs: mitigate slow mfs writes when it triggers (#10623)
* pinmfs: mitigate slow mfs writes when it triggers This mitigates slow mfs writes when the pinmfs daemon calls mfs.RootNode() When writing lots of files to MFS, this call triggers a mfs directory cache sync operations. The cache grows forever and is unbounded. The more files added to a directory, the longer it takes. In the meantime, writing to mfs is locked. The pinmfs, even when no remote pinning services are configured, will trigger this issue. When RootNode() takes more than 30 seconds, the issue will be triggered continuously causing a write-deadlock onto MFS. This commit does not fix the fact that if you write 1M items into an MFS directory, the first time that the directory is traversed it will still have to sync those 1M items. It does at least prevent writes from stalling after about ~6000 items. * pinmfs: fix test
Hector Sanjuan committed
Dec 17, 2024 at 21:59 UTC
898f024f3c94bcb370998b3a7039fa7057de3e32
2 files changed
+47
-22
cmd/ipfs/kubo/pinmfs.go
+31
-19
@@ -90,34 +90,46 @@ func pinMFSOnChange(cctx pinMFSContext, configPollInterval time.Duration, node p
90
case <-cctx.Context().Done():
91
return
92
case <-tmo.C:
93
- tmo.Reset(configPollInterval)
94
- }
95
-
96
- // reread the config, which may have changed in the meantime
97
- cfg, err := cctx.GetConfig()
98
- if err != nil {
99
- mfslog.Errorf("pinning reading config (%v)", err)
100
- continue
101
- }
102
- mfslog.Debugf("pinning loop is awake, %d remote services", len(cfg.Pinning.RemoteServices))
93
+ // reread the config, which may have changed in the meantime
94
+ cfg, err := cctx.GetConfig()
95
+ if err != nil {
96
+ mfslog.Errorf("pinning reading config (%v)", err)
97
+ continue
98
+ }
99
+ mfslog.Debugf("pinning loop is awake, %d remote services", len(cfg.Pinning.RemoteServices))
100
104
- // get the most recent MFS root cid
105
- rootNode, err := node.RootNode()
106
- if err != nil {
107
- mfslog.Errorf("pinning reading MFS root (%v)", err)
108
- continue
101
+ // pin to all remote services in parallel
102
+ pinAllMFS(cctx.Context(), node, cfg, lastPins)
103
}
110
-
111
- // pin to all remote services in parallel
112
- pinAllMFS(cctx.Context(), node, cfg, rootNode.Cid(), lastPins)
104
+ // pinAllMFS may take long. Reset interval only when we are done doing it
105
+ // so that we are not pinning constantly.
106
+ tmo.Reset(configPollInterval)
107
}
108
}
109
110
// pinAllMFS pins on all remote services in parallel to overcome DoS attacks.
117
-func pinAllMFS(ctx context.Context, node pinMFSNode, cfg *config.Config, rootCid cid.Cid, lastPins map[string]lastPin) {
111
+func pinAllMFS(ctx context.Context, node pinMFSNode, cfg *config.Config, lastPins map[string]lastPin) {
112
ch := make(chan lastPin)
113
var started int
114
115
+ // Bail out to mitigate issue below when not needing to do anything.
116
+ if len(cfg.Pinning.RemoteServices) == 0 {
117
+ return
118
+ }
119
+
120
+ // get the most recent MFS root cid.
121
+ // Warning! This can be super expensive.
122
+ // See https://github.com/ipfs/boxo/pull/751
123
+ // and https://github.com/ipfs/kubo/issues/8694
124
+ // Reading an MFS-directory nodes can take minutes due to
125
+ // ever growing cache being synced to unixfs.
126
+ rootNode, err := node.RootNode()
127
+ if err != nil {
128
+ mfslog.Errorf("pinning reading MFS root (%v)", err)
129
+ return
130
+ }
131
+ rootCid := rootNode.Cid()
132
+
133
for svcName, svcConfig := range cfg.Pinning.RemoteServices {
134
if ctx.Err() != nil {
135
break
cmd/ipfs/kubo/pinmfs_test.go
+16
-3
@@ -94,11 +94,24 @@ func TestPinMFSRootNodeError(t *testing.T) {
94
ctx, cancel := context.WithTimeout(context.Background(), 2*testConfigPollInterval)
95
defer cancel()
96
97
+ // need at least one config to trigger
98
+ cfg := &config.Config{
99
+ Pinning: config.Pinning{
100
+ RemoteServices: map[string]config.RemotePinningService{
101
+ "A": {
102
+ Policies: config.RemotePinningServicePolicies{
103
+ MFS: config.RemotePinningServiceMFSPolicy{
104
+ Enable: false,
105
+ },
106
+ },
107
+ },
108
+ },
109
+ },
110
+ }
111
+
112
cctx := &testPinMFSContext{
113
ctx: ctx,
99
- cfg: &config.Config{
100
- Pinning: config.Pinning{},
101
- },
114
+ cfg: cfg,
115
err: nil,
116
}
117
node := &testPinMFSNode{