8000 feat: add parent selector with connection management and capacity control by SouthWest7 · Pull Request #1161 · dragonflyoss/client · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat: add parent selector with connection management and capacity control #1161

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat: add parent selector with connection management and capacity control #1161

wants to merge 5 commits into from
+604 −13

Conversation

SouthWest7
Copy link
Contributor

Description

This PR significantly enhances the parent selector functionality with robust connection management, reference counting, and capacity control mechanisms to improve reliability and resource utilization.

Related Issue

Changes

New Features

  1. Reference Counting

    • Added ref_count: Arc<AtomicU32> to track active tasks using each parent
    • Implemented increment_ref() and decrement_ref() methods for thread-safe reference management
    • Automatic cleanup when reference count reaches zero

  2. Connection Capacity Control

    • Added capacity checking to prevent exceeding configured connection limits
    • Early validation to skip registration if parent count exceeds available capacity
    • Thread-safe connection counting with Arc<AtomicUsize>

  3. Graceful Connection Management

    • Implemented delayed cleanup mechanism with configurable timeout
    • Protection against premature disconnection of active connections
    • Proper gRPC connection shutdown to avoid "connection broken" errors

  4. Enhanced Parent Registration/Unregistration

    • Added register_parent() and unregister_parent() methods
    • Immediate cleanup when reference count reaches zero
    • Batch unregistration with unregister_all_parents()

Improvements

  1. Concurrency Safety

    • Replaced LruCache with DashMap for better concurrent access
    • Atomic operations for reference counting and connection tracking
    • Compare-and-swap operations to prevent race conditions

  2. Resource Management

    • Automatic cleanup of inactive parents from hosts map
    • Cleanup of associated host info and weights
    • Proper shutdown signal handling with tokio::select!

Technical Details

Data Structures

struct ParentSync {
    parent_id: String,
    shutdown: Shutdown,
    ref_count: Arc<AtomicU32>,
    cleanup_timeout: Duration,
    cleanup_in_progress: Arc<AtomicBool>,
}

pub struct ParentSelector {
    config: Arc<Config>,
    active_connections: Arc<AtomicUsize>,
    sync_interval: Duration,
    hosts: Arc<DashMap<String, ParentSync>>,
    // ... other fields
}

Key Algorithms

  1. Reference Counting: Thread-safe increment/decrement with automatic cleanup
  2. Capacity Control: Pre-validation and runtime checking to respect limits
  3. Delayed Cleanup: Configurable timeout before releasing unused connections
  4. Race Condition Prevention: Compare-and-swap operations for cleanup flags

Motivation and Context

Screenshots (if appropriate)

SouthWest7 added 5 commits May 11, 2025 17:01
@SouthWest7
init
aabaf43 
Signed-off-by: southwest <1403572259@qq.com>
@SouthWest7
Merge branch 'main' of https://github.com/dragonflyoss/client into fe…
c6a2b17 
…at/parent-selector

Signed-off-by: southwest <1403572259@qq.com>
@SouthWest7
feat: parent select by wetghts.
125efbc 
Signed-off-by: southwest <1403572259@qq.com>
@SouthWest7
feat: parent selector
e1b93cd 
Signed-off-by: southwest <1403572259@qq.com>
@SouthWest7
merge with main
cf7e28d 
Signed-off-by: southwest <1403572259@qq.com>
@SouthWest7 SouthWest7 requested a review from a team as a code owner May 27, 2025 05:42
@codecov Codecov
Copy link
codecov bot commented May 27, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 207 lines in your changes missing coverage. Please review.

Project coverage is 48.55%. Comparing base (de812d2) to head (cf7e28d).

Files with missing lines Patch % Lines
dragonfly-client/src/resource/parent_selector.rs 0.00% 159 Missing ⚠️
dragonfly-client/src/resource/piece_collector.rs 0.00% 17 Missing ⚠️
dragonfly-client/src/resource/task.rs 0.00% 17 Missing ⚠️
dragonfly-client/src/bin/dfdaemon/main.rs 0.00% 8 Missing ⚠️
dragonfly-client/src/grpc/dfdaemon_upload.rs 0.00% 6 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1161      +/-   ##
==========================================
- Coverage   49.33%   48.55%   -0.79%     
==========================================
  Files          58       59       +1     
  Lines       12114    12311     +197     
==========================================
  Hits         5977     5977              
- Misses       6137     6334     +197
Files with missing lines Coverage Δ
dragonfly-client/src/grpc/dfdaemon_upload.rs 0.00% <0.00%> (ø)
dragonfly-client/src/bin/dfdaemon/main.rs 0.00% <0.00%> (ø)
dragonfly-client/src/resource/piece_collector.rs 0.00% <0.00%> (ø)
dragonfly-client/src/resource/task.rs 9.05% <0.00%> (-0.25%) ⬇️
dragonfly-client/src/resource/parent_selector.rs 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gaius-qi gaius-qi added the enhancement New feature or request label May 27, 2025
gaius-qi
gaius-qi requested changes May 29, 2025
View reviewed changes
dragonfly-client/src/resource/parent_selector.rs
id_generator: Arc<IDGenerator>,

/// hosts_info is the latest host info of different parents.
hosts_info: Arc<DashMap<String, Host>>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge hosts_info and hosts_weights.

dragonfly-client/src/resource/parent_selector.rs
config: Arc<Config>,

/// active_connections tracks the current number of active connections.
active_connections: Arc<AtomicUsize>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refer to https://github.com/dragonflyoss/client/blob/main/dragonfly-client/src/resource/piece_downloader.rs#L92.

dragonfly-client/src/resource/parent_selector.rs

/// ParentSync manages the synchronization of host information from a parent.
#[derive(Clone)]
struct ParentSync {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what this is for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gaius-qi gaius-qi gaius-qi requested changes

@liubin liubin Awaiting requested review from liubin liubin is a code owner automatically assigned from dragonflyoss/dragonfly2-reviewers

@hhhhsdxxxx hhhhsdxxxx Awaiting requested review from hhhhsdxxxx hhhhsdxxxx is a code owner automatically assigned from dragonflyoss/dragonfly2-reviewers

@akashhr akashhr Awaiting requested review from akashhr akashhr is a code owner automatically assigned from dragonflyoss/dragonfly2-reviewers

@CormickKneey CormickKneey Awaiting requested review from CormickKneey

@xujihui1985 xujihui1985 Awaiting requested review from xujihui1985

Requested changes must be addressed to merge this pull request.

Assignees

@SouthWest7 SouthWest7

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SouthWest7 @gaius-qi
0