diff --git a/weed/filer/filer_lazy_remote_test.go b/weed/filer/filer_lazy_remote_test.go index 33e0197f6..673a2eaa2 100644 --- a/weed/filer/filer_lazy_remote_test.go +++ b/weed/filer/filer_lazy_remote_test.go @@ -300,6 +300,19 @@ func registerStubMaker(t *testing.T, storageType string, client remote_storage.R } } +// putConfigEntry stores a filer entry whose content is a serialized configuration. +func putConfigEntry(store *stubFilerStore, path string, content []byte) { + store.entries[path] = &Entry{ + FullPath: util.FullPath(path), + Attr: Attr{ + Mtime: time.Unix(1700000000, 0), + Crtime: time.Unix(1700000000, 0), + Mode: 0644, + }, + Content: content, + } +} + // --- tests --- func TestMaybeLazyFetchFromRemote_HitsRemoteAndPersists(t *testing.T) { @@ -581,6 +594,57 @@ func TestDeleteEntryMetaAndData_IsFromOtherClusterSkipsRemoteDelete(t *testing.T require.Len(t, stub.removeCalls, 0) } +func TestDeleteEntryMetaAndData_UnmountedDirectorySkipsRemoteDelete(t *testing.T) { + const storageType = "stub_lazy_unmounted" + stub := &stubRemoteClient{} + defer registerStubMaker(t, storageType, stub)() + + store := newStubFilerStore() + confContent, err := proto.Marshal(&remote_pb.RemoteConf{Name: "cloud1", Type: storageType}) + require.NoError(t, err) + putConfigEntry(store, DirectoryEtcRemote+"/cloud1"+REMOTE_STORAGE_CONF_SUFFIX, confContent) + mountedContent, err := proto.Marshal(&remote_pb.RemoteStorageMapping{ + Mappings: map[string]*remote_pb.RemoteStorageLocation{ + "/buckets/mybucket": {Name: "cloud1", Bucket: "mybucket", Path: "/"}, + }, + }) + require.NoError(t, err) + putConfigEntry(store, DirectoryEtcRemote+"/"+REMOTE_STORAGE_MOUNT_FILE, mountedContent) + + filePath := util.FullPath("/buckets/mybucket/cached.txt") + store.entries[string(filePath)] = &Entry{ + FullPath: filePath, + Attr: Attr{ + Mtime: time.Unix(1700000000, 0), + Crtime: time.Unix(1700000000, 0), + Mode: 0644, + FileSize: 64, + }, + Remote: &filer_pb.RemoteEntry{RemoteMtime: 1700000000, RemoteSize: 64}, + } + + f := newTestFiler(t, store, NewFilerRemoteStorage()) + f.LoadRemoteStorageConfAndMapping() + _, remoteLoc := f.RemoteStorage.FindMountDirectory(filePath) + require.NotNil(t, remoteLoc) + + unmountedContent, err := proto.Marshal(&remote_pb.RemoteStorageMapping{}) + require.NoError(t, err) + putConfigEntry(store, DirectoryEtcRemote+"/"+REMOTE_STORAGE_MOUNT_FILE, unmountedContent) + f.onMetadataChangeEvent(&filer_pb.SubscribeMetadataResponse{ + Directory: DirectoryEtcRemote, + EventNotification: &filer_pb.EventNotification{ + NewEntry: &filer_pb.Entry{Name: REMOTE_STORAGE_MOUNT_FILE, Content: unmountedContent}, + }, + }) + _, remoteLoc = f.RemoteStorage.FindMountDirectory(filePath) + require.Nil(t, remoteLoc) + + require.NoError(t, f.DeleteEntryMetaAndData(context.Background(), filePath, false, false, true, false, nil, 0)) + require.Len(t, stub.deleteCalls, 0) + require.Len(t, stub.removeCalls, 0) +} + func TestDeleteEntryMetaAndData_RemoteOnlyFileNotUnderMountSkipsRemoteDelete(t *testing.T) { const storageType = "stub_lazy_delete_not_under_mount" stub := &stubRemoteClient{} diff --git a/weed/filer/filer_on_meta_event.go b/weed/filer/filer_on_meta_event.go index 5644e632b..d57810fac 100644 --- a/weed/filer/filer_on_meta_event.go +++ b/weed/filer/filer_on_meta_event.go @@ -136,5 +136,11 @@ func (f *Filer) LoadRemoteStorageConfAndMapping() { } } func (f *Filer) maybeReloadRemoteStorageConfigurationAndMapping(event *filer_pb.SubscribeMetadataResponse) { - // FIXME add reloading + if !filer_pb.MetadataEventTouchesDirectory(event, DirectoryEtcRemote) { + return + } + + // a mount left behind after remote.unmount would still send deletes to a + // remote the filer no longer owns + f.LoadRemoteStorageConfAndMapping() } diff --git a/weed/filer/remote_storage.go b/weed/filer/remote_storage.go index 48096bcd7..121c81518 100644 --- a/weed/filer/remote_storage.go +++ b/weed/filer/remote_storage.go @@ -5,6 +5,7 @@ import ( "fmt" "math" "strings" + "sync" "github.com/seaweedfs/seaweedfs/weed/pb" "github.com/seaweedfs/seaweedfs/weed/pb/remote_pb" @@ -22,6 +23,9 @@ const REMOTE_STORAGE_CONF_SUFFIX = ".conf" const REMOTE_STORAGE_MOUNT_FILE = "mount.mapping" type FilerRemoteStorage struct { + // guards rules and storageNameToConf, which are replaced wholesale + // whenever /etc/remote changes + mu sync.RWMutex rules ptrie.Trie[*remote_pb.RemoteStorageLocation] storageNameToConf map[string]*remote_pb.RemoteConf } @@ -48,44 +52,62 @@ func (rs *FilerRemoteStorage) LoadRemoteStorageConfigurationsAndMapping(filer *F return } + // build into fresh containers so an unmounted directory disappears instead + // of lingering in the trie, which has no way to drop a key + rules := ptrie.New[*remote_pb.RemoteStorageLocation]() + storageNameToConf := make(map[string]*remote_pb.RemoteConf) + for _, entry := range entries { if entry.Name() == REMOTE_STORAGE_MOUNT_FILE { - if err := rs.loadRemoteStorageMountMapping(entry.Content); err != nil { + if err := loadRemoteStorageMountMapping(rules, entry.Content); err != nil { return err } continue } if !strings.HasSuffix(entry.Name(), REMOTE_STORAGE_CONF_SUFFIX) { - return nil + continue } conf := &remote_pb.RemoteConf{} if err := proto.Unmarshal(entry.Content, conf); err != nil { return fmt.Errorf("unmarshal %s/%s: %v", DirectoryEtcRemote, entry.Name(), err) } - rs.storageNameToConf[conf.Name] = conf + storageNameToConf[conf.Name] = conf } + + rs.mu.Lock() + rs.rules, rs.storageNameToConf = rules, storageNameToConf + rs.mu.Unlock() + return nil } -func (rs *FilerRemoteStorage) loadRemoteStorageMountMapping(data []byte) (err error) { +func loadRemoteStorageMountMapping(rules ptrie.Trie[*remote_pb.RemoteStorageLocation], data []byte) (err error) { mappings := &remote_pb.RemoteStorageMapping{} if err := proto.Unmarshal(data, mappings); err != nil { return fmt.Errorf("unmarshal %s/%s: %v", DirectoryEtcRemote, REMOTE_STORAGE_MOUNT_FILE, err) } for dir, storageLocation := range mappings.Mappings { - rs.mapDirectoryToRemoteStorage(util.FullPath(dir), storageLocation) + putDirectoryToRemoteStorage(rules, util.FullPath(dir), storageLocation) } return nil } func (rs *FilerRemoteStorage) mapDirectoryToRemoteStorage(dir util.FullPath, loc *remote_pb.RemoteStorageLocation) { - rs.rules.Put([]byte(dir+"/"), loc) + rs.mu.Lock() + defer rs.mu.Unlock() + putDirectoryToRemoteStorage(rs.rules, dir, loc) +} + +func putDirectoryToRemoteStorage(rules ptrie.Trie[*remote_pb.RemoteStorageLocation], dir util.FullPath, loc *remote_pb.RemoteStorageLocation) { + rules.Put([]byte(dir+"/"), loc) } // FindMountDirectory returns the mount directory and location for p. When multiple // mounts match (e.g. /buckets/b and /buckets/b/prefix), ptrie MatchPrefix visits // shorter prefixes first, so the last match is the longest prefix. func (rs *FilerRemoteStorage) FindMountDirectory(p util.FullPath) (mountDir util.FullPath, remoteLocation *remote_pb.RemoteStorageLocation) { + rs.mu.RLock() + defer rs.mu.RUnlock() rs.rules.MatchPrefix([]byte(p), func(key []byte, value *remote_pb.RemoteStorageLocation) bool { mountDir = util.FullPath(string(key[:len(key)-1])) remoteLocation = value @@ -95,22 +117,18 @@ func (rs *FilerRemoteStorage) FindMountDirectory(p util.FullPath) (mountDir util } func (rs *FilerRemoteStorage) FindRemoteStorageClient(p util.FullPath) (client remote_storage.RemoteStorageClient, remoteConf *remote_pb.RemoteConf, found bool) { - var storageLocation *remote_pb.RemoteStorageLocation - rs.rules.MatchPrefix([]byte(p), func(key []byte, value *remote_pb.RemoteStorageLocation) bool { - storageLocation = value - return true - }) - + _, storageLocation := rs.FindMountDirectory(p) if storageLocation == nil { - found = false - return + return nil, nil, false } return rs.GetRemoteStorageClient(storageLocation.Name) } func (rs *FilerRemoteStorage) GetRemoteStorageClient(storageName string) (client remote_storage.RemoteStorageClient, remoteConf *remote_pb.RemoteConf, found bool) { + rs.mu.RLock() remoteConf, found = rs.storageNameToConf[storageName] + rs.mu.RUnlock() if !found { return } diff --git a/weed/shell/command_remote_unmount.go b/weed/shell/command_remote_unmount.go index bb70744c6..030dd9021 100644 --- a/weed/shell/command_remote_unmount.go +++ b/weed/shell/command_remote_unmount.go @@ -65,7 +65,8 @@ func (c *commandRemoteUnmount) Do(args []string, commandEnv *CommandEnv, writer return fmt.Errorf("directory %s is not mounted", *dir) } - // store a mount configuration in filer + // delete the mount mapping first: the filer reloads it on the spot, so the + // purge below stays local instead of deleting the remote objects fmt.Fprintf(writer, "deleting mount for %s ...\n", *dir) if err = filer.DeleteMountMapping(commandEnv, *dir); err != nil { return fmt.Errorf("delete mount mapping: %w", err)