Skip to content

fix change task status #194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 5, 2025
Merged

fix change task status #194

merged 2 commits into from
Jun 5, 2025

Conversation

dudizimber
Copy link
Collaborator

@dudizimber dudizimber commented Jun 5, 2025

fix make local backup
fix #172

Summary by CodeRabbit

  • New Features
    • Added support for handling the Append Only File (AOF) feature during RDB import and backup operations, allowing the system to distinguish and process instances with AOF enabled.
  • Bug Fixes
    • Task status is now correctly marked as 'completed' after successful local backup deletion.
  • Tests
    • Updated test suites to include scenarios with the AOF feature enabled during RDB import processes.

fix make local backup
Copy link

coderabbitai bot commented Jun 5, 2025

Walkthrough

This change introduces the aofEnabled boolean property throughout the RDB import workflow, updating schemas, processor logic, and test cases to propagate and utilize this flag. The flushInstance and backup logic in the Kubernetes repository are modified to conditionally handle AOF-enabled instances, ensuring correct backup and flush operations based on the instance's configuration.

Changes

File(s) Change Summary
.../schemas/src/services/db-importer-worker/v1/processors/rdb-import.ts Added aofEnabled boolean property to RdbImportFlushInstanceProcessorDataSchema.
.../db-importer-worker/src/processors/RdbImportFlushInstanceProcessor.ts Passes aofEnabled to k8sRepository.flushInstance.
.../db-importer-worker/src/processors/RdbImportDeleteLocalBackupProcessor.ts Updates task status to 'completed' after successful backup deletion.
.../db-importer-worker/src/repositories/k8s/K8sRepository.ts Updates makeLocalBackup to use shell command for AOF/RDB copy; updates flushInstance to handle aofEnabled logic.
.../db-importer-worker/src/tests/import-rdb-cluster.spec.ts Adds aofEnabled to job data in test for cluster import.
.../db-importer-worker/src/tests/import-rdb-replication.spec.ts Adds aofEnabled to job data in test for replication import.
.../db-importer-worker/src/tests/import-rdb-standalone.spec.ts Adds aofEnabled to job data in test for standalone import.
.../db-importer/src/repositories/tasksQueue/TaskQueueBullMQRepository.ts Passes aofEnabled to RdbImportFlushInstance node in job data.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant API
    participant TaskQueue
    participant Worker
    participant K8sRepository

    User->>API: Upload RDB file, specify import options (including aofEnabled)
    API->>TaskQueue: Create import job (includes aofEnabled)
    TaskQueue->>Worker: Start RdbImportFlushInstanceProcessor (job.data includes aofEnabled)
    Worker->>K8sRepository: flushInstance(..., aofEnabled)
    alt aofEnabled == true
        K8sRepository->>K8sRepository: flushall, then sendRewriteAofCommand
    else aofEnabled == false
        K8sRepository->>K8sRepository: flushall, then sendSaveCommand
    end
    Worker->>TaskQueue: Update task status
Loading

Assessment against linked issues

Objective Addressed Explanation
RDB import service supports standalone, sentinel, cluster deployments; wipes DB before import; uses redis-rdb-cli or equivalent (#172)
API and backend propagate/import aofEnabled flag to handle AOF-enabled instances correctly (#172)
Testing plans cover unit/integration/system tests for robustness and correctness (#172)
Progress indicators, error handling, and task status updates in import/export workflow (#172)

Suggested reviewers

  • AviAvni

Poem

In the warren where the data flows,
AOF or RDB—now the system knows!
With a flag for every flush and save,
Backups hop along, robust and brave.
The tests all pass, the carrots cheer—
Another import done, let’s rabbit-ear!
🥕🐇


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f6e2633 and bbc5161.

📒 Files selected for processing (8)
  • backend/libs/schemas/src/services/db-importer-worker/v1/processors/rdb-import.ts (1 hunks)
  • backend/services/db-importer-worker/src/processors/RdbImportDeleteLocalBackupProcessor.ts (1 hunks)
  • backend/services/db-importer-worker/src/processors/RdbImportFlushInstanceProcessor.ts (1 hunks)
  • backend/services/db-importer-worker/src/repositories/k8s/K8sRepository.ts (3 hunks)
  • backend/services/db-importer-worker/src/tests/import-rdb-cluster.spec.ts (1 hunks)
  • backend/services/db-importer-worker/src/tests/import-rdb-replication.spec.ts (1 hunks)
  • backend/services/db-importer-worker/src/tests/import-rdb-standalone.spec.ts (1 hunks)
  • backend/services/db-importer/src/repositories/tasksQueue/TaskQueueBullMQRepository.ts (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (11)
  • GitHub Check: Plan stack observability_stack_ctrl_plane_k8s
  • GitHub Check: Plan stack observability_stack_ctrl_plane_infra
  • GitHub Check: build
  • GitHub Check: build-and-push-docker (db-importer)
  • GitHub Check: build-and-push-docker (db-importer-worker)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (python)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: build-and-push-docker (db-importer)
  • GitHub Check: build
  • GitHub Check: build-and-push-docker (db-importer-worker)
🔇 Additional comments (11)
backend/services/db-importer-worker/src/processors/RdbImportFlushInstanceProcessor.ts (1)

30-30: LGTM: AOF flag properly propagated to flushInstance method.

The addition of job.data.aofEnabled parameter correctly passes the AOF configuration to the flushInstance method, enabling conditional AOF handling during the flush operation.

backend/services/db-importer-worker/src/processors/RdbImportDeleteLocalBackupProcessor.ts (1)

31-34: Excellent fix: Task completion status now properly tracked.

This change addresses the core PR objective by ensuring that successful local backup deletions are properly marked as 'completed' in the task repository. Previously, only failed operations updated the task status, leaving successful operations without proper status tracking.

backend/services/db-importer-worker/src/tests/import-rdb-standalone.spec.ts (1)

132-132: LGTM: Test updated to include AOF parameter.

The test correctly includes the aofEnabled parameter in the job data for RdbImportFlushInstanceProcessor, ensuring test coverage remains complete after the processor changes.

backend/services/db-importer-worker/src/tests/import-rdb-replication.spec.ts (1)

132-132: LGTM: Replication test updated with AOF parameter.

The test correctly includes the aofEnabled parameter, and notably uses aofEnabled: true which provides good test coverage for AOF-enabled scenarios, complementing the standalone test that uses aofEnabled: false.

backend/services/db-importer-worker/src/tests/import-rdb-cluster.spec.ts (1)

132-132: LGTM: Correct propagation of aofEnabled flag to job data.

The addition of aofEnabled: task.payload.aofEnabled ensures the RdbImportFlushInstanceProcessor receives the AOF configuration, aligning the test with the updated schema and implementation.

backend/services/db-importer/src/repositories/tasksQueue/TaskQueueBullMQRepository.ts (1)

332-332: LGTM: Proper propagation of aofEnabled configuration.

Adding aofEnabled: task.payload.aofEnabled to the RdbImportFlushInstance job data ensures the processor can make conditional decisions based on the AOF configuration.

backend/libs/schemas/src/services/db-importer-worker/v1/processors/rdb-import.ts (1)

101-101: LGTM: Schema correctly extended for AOF support.

Adding aofEnabled: Type.Boolean() to the RdbImportFlushInstanceProcessorDataSchema properly defines the expected boolean property for AOF configuration handling.

backend/services/db-importer-worker/src/repositories/k8s/K8sRepository.ts (4)

791-794: LGTM: Correct conditional backup logic for AOF vs RDB.

The shell command properly handles both backup scenarios:

  • AOF enabled: copies the entire /data/appendonlydir directory
  • AOF disabled: copies the single /data/dump.rdb file

The conditional logic ensures the appropriate backup strategy is used based on the instance configuration.


800-800: LGTM: Consistent command execution pattern.

Using ['sh', '-c', shellCommand] is the correct approach for executing the conditional shell command constructed above.


865-865: LGTM: Method signature correctly extended.

Adding the aofEnabled: boolean = false parameter with a sensible default maintains backward compatibility while enabling the new AOF-aware functionality.


878-889: LGTM: Proper promise chaining with conditional post-flush operations.

The implementation correctly:

  • Chains the AOF rewrite or save command after the successful flush operation
  • Uses conditional logic based on the aofEnabled flag
  • Maintains proper error handling by moving the catch to the end of the chain
  • Preserves the original error context in the logging

This ensures that after flushing the instance, the appropriate persistence operation (AOF rewrite or RDB save) is triggered based on the instance configuration.

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • 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/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

github-actions bot commented Jun 5, 2025

Tofu Plan Output - observability_stack_ctrl_plane_infra


�[0m�[1m�[32mNo changes.�[0m�[1m Your infrastructure matches the configuration.�[0m

�[0mOpenTofu has compared your real infrastructure against your configuration and
found no differences, so no changes are needed.

Copy link

github-actions bot commented Jun 5, 2025

Tofu Plan Output - observability_stack_ctrl_plane_k8s


OpenTofu used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
  �[33m~�[0m update in-place�[0m

OpenTofu will perform the following actions:

�[1m  # kubernetes_namespace.api�[0m will be updated in-place
�[0m  �[33m~�[0m�[0m resource "kubernetes_namespace" "api" {
        id                               = "api"
        �[90m# (1 unchanged attribute hidden)�[0m�[0m

      �[33m~�[0m�[0m metadata {
          �[33m~�[0m�[0m labels           = {
              �[31m-�[0m�[0m "argocd.argoproj.io/instance" = "observability-stack" �[90m-> null�[0m�[0m
            }
            name             = "api"
            �[90m# (4 unchanged attributes hidden)�[0m�[0m
        }
    }

�[1mPlan:�[0m 0 to add, 1 to change, 0 to destroy.
�[0m

@dudizimber dudizimber merged commit 5d79775 into main Jun 5, 2025
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create RDB import service
2 participants