TrueCloudLab/frostfs-sdk-go

Fork 21

Move connection management from `Pool` to `ConnectionManager` #300

New issue

Closed

opened 2024-12-03 10:24:18 +00:00 by achuprov · 6 comments

achuprov commented

2024-12-03 10:24:18 +00:00

Member

The Pool has mixed responsibilities. It handles both FrostFS operations and connection management (like health checks and work with innerPool).
Create ConnectionManager and move connection management to ConnectionManager. This is needed for #30.

The `Pool` has mixed responsibilities. It handles both `FrostFS` operations and connection management (like health checks and work with `innerPool`). Create `ConnectionManager` and move connection management to `ConnectionManager`. This is needed for #30.

achuprov added the

internal

label 2024-12-03 10:24:35 +00:00

dkirillov commented

2024-12-03 11:12:15 +00:00

Member

It would be nice to see high level overview of proposed changes (in go or pseudo code) before starting developing.
In general I like idea of simplifying Pool

It would be nice to see high level overview of proposed changes (in go or pseudo code) before starting developing. In general I like idea of simplifying `Pool`

dkirillov commented

2024-12-03 11:13:09 +00:00

Member

By the way, we had #204. Why don't we implement this idea there?

By the way, we had https://git.frostfs.info/TrueCloudLab/frostfs-sdk-go/pulls/204. Why don't we implement this idea there?

fyrchik commented

2024-12-03 12:42:42 +00:00

Owner

Why don't we implement this idea there?

Because it is already too big and old.
I would rather first see the proposed changes, as you also suggested.

>Why don't we implement this idea there? Because it is already too big and old. I would rather first see the proposed changes, as you also suggested.

achuprov commented

2024-12-03 13:57:23 +00:00

Author

Member

I propose the following structure:

type innerPool struct {
	lock    sync.RWMutex
	sampler *sampler
	clients []client
}


type ConnectionManager struct {
	Cache           *SessionCache
	lock            sync.RWMutex
	innerPools      []*innerPool
	cancel          context.CancelFunc
	closedCh        chan struct{}
	rebalanceParams rebalanceParameters
	key            *ecdsa.PrivateKey
	stokenDuration uint64
	
	clientBuilder clientBuilder
}

func (m *ConnectionManager) Connecion() client
func (m *ConnectionManager) SetConfig([]*nodesParam)
func (m *ConnectionManager) Close()

type Pool struct {
	Manager        *ConnectionManager
	maxObjectSize uint64
}

I propose the following structure: ``` type innerPool struct { lock sync.RWMutex sampler *sampler clients []client } type ConnectionManager struct { Cache *SessionCache lock sync.RWMutex innerPools []*innerPool cancel context.CancelFunc closedCh chan struct{} rebalanceParams rebalanceParameters key *ecdsa.PrivateKey stokenDuration uint64 clientBuilder clientBuilder } func (m *ConnectionManager) Connecion() client func (m *ConnectionManager) SetConfig([]*nodesParam) func (m *ConnectionManager) Close() type Pool struct { Manager *ConnectionManager maxObjectSize uint64 } ```

fyrchik added the

discussion

refactoring

pool

labels 2024-12-03 14:14:31 +00:00

fyrchik commented

2024-12-05 13:28:26 +00:00

Owner

@achuprov could you also describe what are the responsibilities of each struct and what methods do they have.
It is not obvious rn.

@achuprov could you also describe what are the responsibilities of each struct and what methods do they have. It is not obvious rn.

achuprov commented

2024-12-17 06:37:26 +00:00

Author

Member

@fyrchik The Pool will still include Deal, Close, and all the currently public methods.

The ConnectionManager will provide methods like Deal, Close, as well as connection(), Statistic(), and all the helper methods used in the Pool, such as checkSessionTokenErr, initCallContext, openDefaultSession, call, fillAppropriateKey, and others.

Additionally, I suggest moving the logic for managing goroutines into HealthCheck. It will handle checking the health of clients in the innerPool using a callback function.

I don’t intend to drastically refactor the structure of the code—I simply want to separate the relevant logic into specific components

@fyrchik The `Pool` will still include `Deal`, `Close`, and all the currently public methods. The `ConnectionManager` will provide methods like `Dea`l, `Close`, as well as `connection()`, `Statistic()`, and all the helper methods used in the `Pool`, such as `checkSessionTokenErr`, `initCallContext`, `openDefaultSession`, `call`, `fillAppropriateKey`, and others. Additionally, I suggest moving the logic for managing goroutines into `HealthCheck`. It will handle checking the health of clients in the `innerPool` using a callback function. I don’t intend to drastically refactor the structure of the code—I simply want to separate the relevant logic into specific components

achuprov referenced this issue from a pull request that will close it,

2024-12-18 16:19:01 +00:00

Refactor 'Pool' #316

achuprov was assigned by fyrchik

2025-02-07 13:24:28 +00:00

fyrchik referenced this issue from a commit

2025-03-07 11:45:32 +00:00

[#300] pool: Extract connection handler functionality to 'connectionManager'

fyrchik referenced this issue from a commit

2025-03-07 11:45:33 +00:00

[#300] pool: Extract healthCheck functionality from 'connectionManager' to 'healthCheck'

fyrchik referenced this issue from a commit