Ivan386/filestore fix (#7474)

* ipfs filestore fix [<obj>]... - 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

@@ -28,6 +28,7 @@ var FileStoreCmd = &cmds.Command{
 
 const (
 	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:
 
-<status> <hash> <size> <path> <offset>
+<status> <hash> <size> <path> <offset> [<action>]
 
 Where <status> is one of:
 ok:       the block can be reconstructed
@@ -118,6 +119,10 @@ error:    there was some other problem reading the file
 missing:  <obj> could not be found in the filestore
 ERROR:    internal error, most likely due to a corrupt database
 
+Where <action> 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,8 +196,17 @@ For ERROR entries the error will also be printed to stderr.
 				if list.Status == filestore.StatusOtherError {
 					fmt.Fprintf(os.Stderr, "%s\n", list.ErrorMsg)
 				}
+
+				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))
 				}
+			}
 		},
 	},
 	Type: filestore.ListRes{},
@@ -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
 		}
 	}

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)

test/sharness/t0271-filestore-utils.sh

@@ -63,6 +63,20 @@ EOF
 
 sort < verify_expect_file_order > verify_expect_key_order
 
+cat <<EOF > 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 <<EOF > 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