feat(fisherman): 🌎 batch file deletions #531
Open
+5,504
−4,104
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Move file deletion logic from StorageRequestRevoked to IncompleteStorageRequest handler to establish single source of truth for incomplete storage cleanup. Previously, StorageRequestRevoked handled all deletion logic. Now it only deletes files with no provider associations (when revoked before any confirmations). Files with associations are handled by IncompleteStorageRequest event which the runtime conditionally emits only when providers have confirmed storage.
Add database query methods to support fisherman batch deletion processing. These methods enable efficient querying of files pending deletion, grouped by target (BSP or Bucket), for the upcoming batch deletion coordinator. - Add FileDeletionType enum to distinguish user vs incomplete deletions - Add get_pending_user_deletions and get_pending_incomplete_deletions - Add get_files_pending_deletion_grouped_by_bucket - Add get_files_pending_deletion_grouped_by_bsp - Add DEFAULT_FILE_QUERY_LIMIT (100) and DEFAULT_BATCH_QUERY_LIMIT (1000) - Add Eq and Hash derives to OnchainBspId for HashMap key usage
Add database indexes to optimize fisherman batch deletion queries. These indexes support efficient filtering, joining, and ordering for get_files_pending_deletion_grouped_by_bsp/bucket methods. - idx_bsp_onchain_bsp_id: Filter BSPs by onchain ID - idx_file_onchain_bucket_id: Filter and order files by bucket ID - idx_file_deletion_bucket_order: Composite partial index for bucket-grouped deletion queries with ordering
…to task module - Remove GetPendingDeletions command from fisherman-service - Move query logic to get_pending_deletions() in fisherman_process_file_deletion.rs - Remove indexer_db_pool from FishermanService (will be accessed via StorageHubHandler instead) - Simplifies architecture by eliminating unnecessary actor commands
snowmead
changed the title
GetPendingDeletions command…unning as fisherman role
…man processing - Introduced `waitForFishermanBatchDeletions` utility to synchronize test execution with fisherman processing for both user and incomplete deletions. - Updated multiple test files to utilize the new utility, ensuring that deletion events are properly awaited before assertions. - Refactored deletion request handling in tests to build and seal blocks for batch deletions and revocations more efficiently.
- indexing IncompleteStorageRequest in fishing handler - refactored indexing apis across the board - streamlined batching file creation with BSP and MSP acceptance - use appropriate apis for fetching container names
…letions` properly
- Simplified the verification of BSP and bucket deletion results by introducing dedicated functions `verifyBspDeletionResults` and `verifyBucketDeletionResults`. - Updated tests to utilize these new functions for better readability and maintainability. - Ensured that the expected number of deletion events and forest root changes are properly validated in the tests.
- Updated the chunk writing logic in both in-memory and RocksDB implementations to improve error handling and logging. - Introduced a unified approach for checking file completion status, ensuring consistency across different storage backends. - Added a new error variant for failed completion checks in the FileStorageWriteError enum. - Enhanced logging with debug statements to provide better insights during chunk operations. - Use `is_file_complete` api everywhere instead of manual checks
snowmead
changed the title
snowmead
changed the title
- batch storage requests api - retry fisherman batch deletion check api
snowmead
force-pushed
the
feat/fisherman-service-batch-commands
branch
from
acb8a6c to
f85ceae
Compare
Resolve modify/delete conflict by keeping fisherman_process_file_deletion.rs deleted, as it has been replaced by the batch deletion implementation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
snowmead
added
B5-clientnoteworthy
- insert many files at once with forest storage api - improve variable naming and docs - use waitFor for is file in forest and in file storage checks for batch deletion
…ocessed - update the last_indexed_finalized_block only when all events have been indexed, not before. - indexer api waitForIndexing now waits for last_indexed_finalized_block to be the expected block.
use waitFor and delete helper functions `fileDeletionFromFileStorage` and `mspBucketFileDeletionCompleted`
MSP nodes now require --provider-database-url for move bucket operations (required by CLI since a7d853b). This fix ensures: 1. MSPs always get --provider-database-url in fullnet (regardless of indexer setting) 2. Postgres always starts for fullnet (MSPs need database access) 3. Database migrations run for fullnet (MSPs need schema) 4. Standalone indexer gets --chain=solochain-evm-dev flag for solochain runtime This fixes widespread test failures across User, BSPNet, FullNet, Solochain EVM, Fisherman, and Backend integration test suites where MSP nodes failed to start without database configuration. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
MSP nodes now perform database connection and migrations during startup (since a7d853b), which adds a delay before the RPC server is fully ready. This commit adds explicit waits for MSP nodes to show "💤 Idle" logs before attempting to fetch their peer IDs via RPC. This fixes the race condition where getPeerId would fail with "Error fetching peerId from http://127.0.0.1:9777" when called before the MSP RPC server was ready, causing the first test in a suite to fail while subsequent tests passed. The fix follows the same pattern already used for BSP and user nodes (lines 874-878 and 894-898). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
MSP nodes have SH_INDEXER_DB_AUTO_MIGRATE enabled by default and handle their own database migrations internally during startup. External migrations using the diesel CLI should only run for user nodes with embedded indexer (which have auto-migrate disabled via SH_INDEXER_DB_AUTO_MIGRATE="false"). This fixes CI test failures where diesel CLI is not installed on the runner, causing "Error running Diesel CLI" when fullnet tests try to run external migrations that are redundant (MSPs already migrated themselves). Before: External migrations ran for fullnet OR indexer After: External migrations only run for indexer 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Add step-by-step logging throughout remapComposeYaml() method - Log configuration changes for runtime type, fisherman, indexer, storage, etc. - Add logging to service startup, migration, and setup methods - Include detailed logs for BSP/MSP setup, runtime params, and demo requests - All logs prefixed with [NetworkLauncher] for easy filtering
… if the available capacity is sufficient on chain
…ally even when available capacity is sufficient on chain (bsp and msp)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The new fisherman task will process batches of either User Requested File Deletions (URFD) or Incomplete Storage Requests (ISR).
The
BatchFileDeletionsevent is emitted by the Fisherman service after every configured interval of time (fisherman_batch_interval_seconds- new cli option), interchanging batch processing between URFDs and ISRs.Files are now only deleted if they are marked for deletion in the indexer database (previously the fisherman would delete files that were signalled via runtime events, unfinalized events).
Notable changes
Node
FishermanTaskandBatchFileDeletionsevent handler.--batch-interval-seconds: Determines the amount of time to wait to process the next batch of file deletions (defaults to 60 seconds).--fisherman-batch-deletion-limit: Determines the maximum number of files to process per batch deletion cycle.is_file_completefunction when writing chunks.--provider-database-urlwhich is independent from running the indexer service via the--indexerflag and its--indexer-datbase-url.Integration Tests
batch-file-deletion-catchup.test.tsandbatch-file-deletion.test.ts.batchStorageRequestsfor our integration tests which simplifies the process of creating an arbitrary amount of storage requests and waiting forBSP 1andMSP 1to have confirmed and accepted all of them respectively.Fixes 🐛
--max-storage-capacity. Previously this check was skipped if the available capacity on chain was sufficient to store the file/bucket.--indexerflag.fishingmode.get_files_pending_deletion_grouped_by_bucket) was returning all file keys to be deleted for a Bucket ID, including ones which did not have an MSP association. This would result in the fisherman constructing an invalid MSP Forest since file keys without MSP associations in the database means they are no longer stored by that MSP (and registered by the runtime).Fishermanrole previously did notexpecttheindexer_db_poolto be present.last_indexed_finalized_blockonly when all events have been indexed, not before.waitForIndexingnow waits forlast_indexed_finalized_blockto be the expected block.Required CLI option for running MSP nodes:
--provider-database-urlRemoved Fisherman CLI options:
--fisherman-incomplete-sync-max--fisherman-incomplete-sync-page-size--fisherman-sync-mode-min-blocks-behind