From d39cb7f57554e2a41506b6022fa27dd58cb6215d Mon Sep 17 00:00:00 2001 From: Ivan Date: Thu, 5 Jun 2025 00:04:20 +0300 Subject: [PATCH] Ivan386/filestore fix (#7474) * ipfs filestore fix []... - Verify objects in filestore and remove bad links * Option --remove-bad-blocks for 'ipfs filestore verify' * fix call to DeleteBlock * action of --remove-bad-blocks in output only when option specified * Add sharness test for removing bad blocks --------- Co-authored-by: Andrew Gillis <11790789+gammazero@users.noreply.github.com> --- core/commands/filestore.go | 49 +++++++++++++++++++++----- docs/changelogs/v0.36.md | 7 +++- test/sharness/t0271-filestore-utils.sh | 43 ++++++++++++++++++++++ 3 files changed, 90 insertions(+), 9 deletions(-) diff --git a/core/commands/filestore.go b/core/commands/filestore.go index 0c9dbee0a..92a20176e 100644 --- a/core/commands/filestore.go +++ b/core/commands/filestore.go @@ -27,7 +27,8 @@ var FileStoreCmd = &cmds.Command{ } const ( - fileOrderOptionName = "file-order" + fileOrderOptionName = "file-order" + removeBadBlocksOptionName = "remove-bad-blocks" ) var lsFileStore = &cmds.Command{ @@ -57,7 +58,7 @@ The output is: } args := req.Arguments if len(args) > 0 { - return listByArgs(req.Context, res, fs, args) + return listByArgs(req.Context, res, fs, args, false) } fileOrder, _ := req.Options[fileOrderOptionName].(bool) @@ -108,7 +109,7 @@ otherwise verify all objects. The output is: - + [] Where is one of: ok: the block can be reconstructed @@ -118,6 +119,10 @@ error: there was some other problem reading the file missing: could not be found in the filestore ERROR: internal error, most likely due to a corrupt database +Where is present only when removing bad blocks and is one of: +remove: link to the block will be removed from datastore +keep: keep link, nothing to do + For ERROR entries the error will also be printed to stderr. `, }, @@ -126,15 +131,18 @@ For ERROR entries the error will also be printed to stderr. }, Options: []cmds.Option{ cmds.BoolOption(fileOrderOptionName, "verify the objects based on the order of the backing file"), + cmds.BoolOption(removeBadBlocksOptionName, "remove bad blocks. WARNING: This may remove pinned data. You should run 'ipfs pin verify' after running this command and correct any issues."), }, Run: func(req *cmds.Request, res cmds.ResponseEmitter, env cmds.Environment) error { _, fs, err := getFilestore(env) if err != nil { return err } + + removeBadBlocks, _ := req.Options[removeBadBlocksOptionName].(bool) args := req.Arguments if len(args) > 0 { - return listByArgs(req.Context, res, fs, args) + return listByArgs(req.Context, res, fs, args, removeBadBlocks) } fileOrder, _ := req.Options[fileOrderOptionName].(bool) @@ -148,7 +156,14 @@ For ERROR entries the error will also be printed to stderr. if r == nil { break } - if err := res.Emit(r); err != nil { + + if removeBadBlocks && (r.Status != filestore.StatusOk) && (r.Status != filestore.StatusOtherError) { + if err = fs.FileManager().DeleteBlock(req.Context, r.Key); err != nil { + return err + } + } + + if err = res.Emit(r); err != nil { return err } } @@ -162,6 +177,8 @@ For ERROR entries the error will also be printed to stderr. return err } + req := res.Request() + removeBadBlocks, _ := req.Options[removeBadBlocksOptionName].(bool) for { v, err := res.Next() if err != nil { @@ -179,7 +196,16 @@ For ERROR entries the error will also be printed to stderr. if list.Status == filestore.StatusOtherError { fmt.Fprintf(os.Stderr, "%s\n", list.ErrorMsg) } - fmt.Fprintf(os.Stdout, "%s %s\n", list.Status.Format(), list.FormatLong(enc.Encode)) + + if removeBadBlocks { + action := "keep" + if removeBadBlocks && (list.Status != filestore.StatusOk) && (list.Status != filestore.StatusOtherError) { + action = "remove" + } + fmt.Fprintf(os.Stdout, "%s %s %s\n", list.Status.Format(), list.FormatLong(enc.Encode), action) + } else { + fmt.Fprintf(os.Stdout, "%s %s\n", list.Status.Format(), list.FormatLong(enc.Encode)) + } } }, }, @@ -236,7 +262,7 @@ func getFilestore(env cmds.Environment) (*core.IpfsNode, *filestore.Filestore, e return n, fs, err } -func listByArgs(ctx context.Context, res cmds.ResponseEmitter, fs *filestore.Filestore, args []string) error { +func listByArgs(ctx context.Context, res cmds.ResponseEmitter, fs *filestore.Filestore, args []string, removeBadBlocks bool) error { for _, arg := range args { c, err := cid.Decode(arg) if err != nil { @@ -250,7 +276,14 @@ func listByArgs(ctx context.Context, res cmds.ResponseEmitter, fs *filestore.Fil continue } r := filestore.Verify(ctx, fs, c) - if err := res.Emit(r); err != nil { + + if removeBadBlocks && (r.Status != filestore.StatusOk) && (r.Status != filestore.StatusOtherError) { + if err = fs.FileManager().DeleteBlock(ctx, r.Key); err != nil { + return err + } + } + + if err = res.Emit(r); err != nil { return err } } diff --git a/docs/changelogs/v0.36.md b/docs/changelogs/v0.36.md index 0cb35085f..09369bcea 100644 --- a/docs/changelogs/v0.36.md +++ b/docs/changelogs/v0.36.md @@ -10,7 +10,8 @@ This release was brought to you by the [Shipyard](http://ipshipyard.com/) team. - [Overview](#overview) - [๐Ÿ”ฆ Highlights](#-highlights) - - [Update go-log to v2](#update-go-log-to-v2) + - [Update go-log to v2](#update-go-log-to-v2 + - [Option for filestore command to remove bad blocks](#option-for-filestore-command-to-remove-bad-blocks) - [๐Ÿ“ฆ๏ธ Important dependency updates](#-important-dependency-updates) - [๐Ÿ“ Changelog](#-changelog) - [๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ Contributors](#-contributors) @@ -28,6 +29,10 @@ go-log v2 has been out for quite a while now and it is time to deprecate v1. - Fixes `ipfs log tail` - Removes support for `ContextWithLoggable` as this is not needed for tracing-like functionality +#### Option for filestore command to remove bad blocks + +The `filestore` command has a new option, `--remove-bad-blocks`, to verify objects in the filestore and remove those that fail verification. + #### ๐Ÿ“ฆ๏ธ Important dependency updates - update `go-libp2p-kad-dht` to [v0.33.0](https://github.com/libp2p/go-libp2p-kad-dht/releases/tag/v0.33.0) diff --git a/test/sharness/t0271-filestore-utils.sh b/test/sharness/t0271-filestore-utils.sh index 5fd335659..5f7111bdd 100755 --- a/test/sharness/t0271-filestore-utils.sh +++ b/test/sharness/t0271-filestore-utils.sh @@ -63,6 +63,20 @@ EOF sort < verify_expect_file_order > verify_expect_key_order +cat < verify_rm_expect +ok bafkreic2wqrsyr3y3qgzbvufen2w25r3p3zljckqyxkpcagsxz3zdcosd4 10000 somedir/file2 0 keep +ok bafkreidx7ivgllulfkzyoo4oa7dfrg4mjmudg2qgdivoooj4s7lh3m5nqu 1000 somedir/file1 0 keep +changed bafkreiemzfmzws23c2po4m6deiueknqfty7r3voes3e3zujmobrooc2ngm 262144 somedir/file3 0 remove +changed bafkreifjcthslybjizk36xffcsb32fsbguxz3ptkl7723wz4u3qikttmam 213568 somedir/file3 786432 remove +changed bafkreigl2pjptgxz6cexcnua56zc5dwsyrc4ph2eulmcb634oes6gzvmuy 262144 somedir/file3 524288 remove +changed bafkreihgm53yhxn427lnfdwhqgpawc62qejog7gega5kqb6uwbyhjm47hu 262144 somedir/file3 262144 remove +EOF + +cat < verify_after_rm_expect +ok bafkreic2wqrsyr3y3qgzbvufen2w25r3p3zljckqyxkpcagsxz3zdcosd4 10000 somedir/file2 0 +ok bafkreidx7ivgllulfkzyoo4oa7dfrg4mjmudg2qgdivoooj4s7lh3m5nqu 1000 somedir/file1 0 +EOF + IPFS_CMD="ipfs" test_filestore_adds() { @@ -155,6 +169,27 @@ test_filestore_verify() { test_init_dataset } +test_filestore_rm_bad_blocks() { + test_filestore_state + + test_expect_success "change first bit of file" ' + dd if=/dev/zero of=somedir/file3 bs=1024 count=1 + ' + + test_expect_success "'$IPFS_CMD filestore verify --remove-bad-blocks' shows changed file removed" ' + $IPFS_CMD filestore verify --remove-bad-blocks > verify_rm_actual && + test_cmp verify_rm_expect verify_rm_actual + ' + + test_expect_success "'$IPFS_CMD filestore verify' shows only files that were not removed" ' + $IPFS_CMD filestore verify > verify_after && + test_cmp verify_after_rm_expect verify_after + ' + + # reset the state for the next test + test_init_dataset +} + test_filestore_dups() { # make sure the filestore is in a clean state test_filestore_state @@ -179,6 +214,8 @@ test_filestore_verify test_filestore_dups +test_filestore_rm_bad_blocks + # # With daemon # @@ -197,6 +234,8 @@ test_filestore_dups test_kill_ipfs_daemon +test_filestore_rm_bad_blocks + ## ## base32 ## @@ -243,6 +282,8 @@ test_filestore_verify test_filestore_dups +test_filestore_rm_bad_blocks + # # With daemon # @@ -263,6 +304,8 @@ test_kill_ipfs_daemon test_done +test_filestore_rm_bad_blocks + ## test_done