feat: remove legacy orphans background

cool-develope commented 6 months ago

Closes: #862

Context

refactor removing the legacy orphans in background.
add the mutex in BatchWithFlusher to avoid the race condition

coderabbitai[bot] commented 6 months ago

Walkthrough

The updates involve refactoring the pruning logic of a Go application's V1. The changes focus on improving the handling of orphan nodes by introducing a new method to delete them and altering the existing pruning function to work asynchronously. This approach aims to enhance performance by preventing the main thread I/O from being blocked, and it also adds the flexibility to manually trigger the pruning process via an exposed API.

Changes

File(s)	Summary of Changes
`nodedb.go`	- Added `time` package import - Added `deleteOrphans` method - Modified `deleteLegacyVersions` to use a goroutine and invoke `deleteOrphans`
`batch.go`	- Introduced `sync.Mutex` field `mtx` in the `BatchWithFlusher` struct - Modified `Set` and `Delete` methods to acquire and release the mutex for thread safety

Assessment against linked issues

Objective	Addressed	Explanation
Refactor the pruning logic to handle orphan nodes more efficiently and allow manual triggering (Issue #862)	✅	The addition of `deleteOrphans` and the modification of `deleteLegacyVersions` to work asynchronously with a goroutine align with the objective of improving efficiency and allowing manual operation.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ?

Tips

### Chat with CodeRabbit Bot (`@coderabbitai`) - You can directly reply to a review comment made by CodeRabbit. Example: - _I pushed a fix in commit ``._ - You can tag CodeRabbit on specific lines of code or entire files in the PR by tagging `@coderabbitai` in a comment. Examples: - _@coderabbitai generate unit tests for this file_ - _@coderabbitai modularize this function_ - You can tag `@coderabbitai` in a PR comment and ask questions about the PR and the codebase. Use quoted replies to pass the context for follow-up questions. Examples: - _@coderabbitai render interesting statistics about this repository as a table_. - _@coderabbitai show all the `console.log` statements in this repository_. - _@coderabbitai generate unit tests for the `src/utils.ts` file_. ### CodeRabbit Commands (invoked as PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger a review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai help` to get help. Additionally, you can add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. ### CodeRabbit Configration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - The JSON schema for the configuration file is available [here](https://coderabbit.ai/integrations/coderabbit-overrides.v2.json). - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json` ### CodeRabbit Discord Community Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback.

ValarDragon commented 6 months ago

Is this concurrency safe with main thread commits? (This can add to deleting while the main thread is committing right?)

Do we need a mutex to ensure the two don't try to add to batch at the same time?

cool-develope commented 6 months ago

Is this concurrency safe with main thread commits? (This can add to deleting while the main thread is committing right?)

Do we need a mutex to ensure the two don't try to add to batch at the same time?

yeah, the test is failed with data racing

ValarDragon commented 6 months ago

This is the wrong spot to put it, we hacked something that worked in this branch https://github.com/cosmos/iavl/tree/dev/v1.0.0-orphan

(the first part of the loop is what was expensive)

I think the way you were doing the code is cleaner, just that was something that I got to work. Plz feel free to takeover/move anything that helps :pray:

cool-develope commented 4 months ago

close in favor of #877

cosmos / iavl