feat: bucket migration state improvement #1250

jingjunLi · 2023-11-15T13:20:22Z

Description

Optimizations for bucket migration:

Persistence of Quota State
Optimization of state persistence in DB for each phase of bucket migration
Bucket Migration Progress Query
Confirming Complete Event & RejectMigrateBucketEvent
Bucket Migration GC Trigger

Feat 1: Persistence of Quota State

In the previous setup, the source storage provider (Src SP) utilized an in-memory map (ManageModular::migratingBuckets) to keep track of whether the quota for bucket migration had been deducted. However, this approach presented challenges in scenarios where the source storage provider experienced an outage and needed to restart, making it difficult to maintain accurate state. To address this issue, the in-memory map has been replaced with a persistent MySQL table.

Abstract some logic from preMigrateBucketHandler and postMigrateBucketHandler into Manager::NotifyPreMigrateBucketAndDeductQuota & Manager::NotifyPostMigrateBucketAndRecoupQuota.

1. After Src SP deducts Quota, the corresponding state is persisted in the database, and a check is performed. This mainly involves the deducted quota amount and the recouped quota amount.
1. Src SP recoups Quota.

feat 2: Optimization of state persistence in DB for each phase of bucket migration

Introduction of the MigrateBucketProgressTable table to record the state and meta of bucket migration. Additionally, encapsulation of interfaces for modifying state and meta.

feat 3: Bucket Migration Progress Query

In both MigrateGVGUnitMeta (core/spdb/entity.go) and MigrateGVGTable, add a field named MigratedBytesSize.

Upon each successful migration, accumulate the MigratedBytesSize from the task. When reporting and calling updateMigrateGVGStatus during the update, use UpdateMigrateGVGMigratedBytesSize to update the MigratedBytesSize field.
Users can query the progress of all bucket migrations through listExecutePlan.

Rationale

Example

NA

Changes

Notable changes:

Notification from Destination SP to Source SP - PreMigrateBucket:
In the processing flow (preMigrateBucketHandler) at the source SP upon receiving notification:

Query the MigrateBucketTable for the MigrateStatus status. If the quota has already been deducted, return "ok" directly.
If quota deduction is pending, invoke DeductQuotaForBucketMigrate to deduct the quota.
After quota deduction, call NotifyDonePreMigrateBucket to update the MigrateStatus status in the MigrateBucketTable to "PreNotified."

Notification from Destination SP to Source SP - PostMigrateBucket:

PostMigrateBucket can only be invoked once.

Potential Impacts

add potential impacts for other components here
...

sysvm

LGTM

base/gfspapp/manage_server.go

will-2012 · 2023-12-13T12:23:35Z

base/gfspclient/interface.go

-	NotifyPostMigrateBucket(ctx context.Context, bmInfo *gfsptask.GfSpBucketMigrationInfo) error
+	GetMigrationBucketState(ctx context.Context, bucketID uint64) (*gfspserver.MigrateBucketProgressMeta, error)
+	NotifyPostMigrateBucketAndRecoupQuota(ctx context.Context, bmInfo *gfsptask.GfSpBucketMigrationInfo) (*gfsptask.GfSpBucketQuotaInfo, error)
+	NotifyPreMigrateBucketAndDeductQuota(ctx context.Context, bucketID uint64) (*gfsptask.GfSpBucketQuotaInfo, error)


Please try to keep a consistent style.

will-2012 · 2023-12-13T12:25:16Z

base/gfspclient/interface.go

@@ -127,6 +128,7 @@ type MetadataAPI interface {
 	ListGlobalVirtualGroupsByBucket(ctx context.Context, bucketID uint64, opts ...grpc.DialOption) ([]*virtualgrouptypes.GlobalVirtualGroup, error)
 	ListGlobalVirtualGroupsBySecondarySP(ctx context.Context, spID uint32, opts ...grpc.DialOption) ([]*virtualgrouptypes.GlobalVirtualGroup, error)
 	ListMigrateBucketEvents(ctx context.Context, blockID uint64, spID uint32, opts ...grpc.DialOption) ([]*types.ListMigrateBucketEvents, error)
+	ListCompleteMigrationBucketEvents(ctx context.Context, blockID uint64, srcSpID uint32, opts ...grpc.DialOption) ([]*storagetypes.EventCompleteMigrationBucket, error)


Is it possible to distinguish the difference between ListMigrateBucketEvents and ListCompleteMigrationBucketEvents through naming?

The semantics of the interfaces are currently a bit different:

ListMigrateBucketEvents returns all events between (0-height), while ListCompleteMigrationBucketEvents returns the current block height.

ListMigrateBucketEvents does not return CompleteMigrationBucketEvent. ListCompleteMigrationBucketEvents is mainly used for the src SP to listen for CompleteMigrationBucketEvent.

ListMigrateBucketEvents has an internal table, EventMigrationBucket, using (dst_primary_sp_id). There is no src_sp_id here, making the implementation a bit more complex.

Considering the above reasons, ListCompleteMigrationBucketEvents has been implemented."

src sp , dest sp comment

will-2012 · 2023-12-13T12:26:59Z

base/types/gfsptask/migrate_gvg.go

@@ -373,3 +373,38 @@ func (m *GfSpGCBucketMigrationTask) GetBucketID() uint64 {
 func (m *GfSpGCBucketMigrationTask) SetBucketID(bucketID uint64) {
 	m.BucketId = bucketID
 }
+
+func (m *GfSpGCBucketMigrationTask) SetLastGCObjectID(lastGCObjectID uint64) {
+	m.LastGcObjectId = lastGCObjectID


Gc or GC? keep a consistent style.

will-2012 · 2023-12-13T13:04:18Z

store/sqldb/migrate_schema.go

+// used by persist bucket migration progress and meta
+type MigrateBucketProgressTable struct {
+	BucketID              uint64 `gorm:"primary_key"`
+	SubscribedBlockHeight uint64 `gorm:"primary_key"`


What is the meaning of the two primary keys in gorm?

100 -> xx init, migrate bucket process
+++ 101 102;
103 -> migrate finished;
add more comment;

base/gfspclient/manager.go

will-2012 · 2023-12-13T13:08:51Z

core/spdb/entity.go

@@ -119,8 +119,9 @@ type MigrateGVGUnitMeta struct {
 	SrcSPID                  uint32
 	DestSPID                 uint32
 	LastMigratedObjectID     uint64
-	MigrateStatus            int // scheduler assign unit status.
-	RetryTime                int //
+	MigrateStatus            int    // scheduler assign unit status.


Status or state, pls distinguish semantics and be consistent.

this is the migrate gvg task 's migrate status

will-2012 · 2023-12-13T13:11:14Z

modular/executor/execute_gc.go

@@ -384,13 +384,16 @@ func (e *ExecuteModular) gcMetaReadRecord(ctx context.Context, task coretask.GCM
 }

 func (e *ExecuteModular) HandleGCBucketMigrationBucket(ctx context.Context, task coretask.GCBucketMigrationTask) {


Whether to support continuing gc from the last breakpoint.

parallel

resumable gc, breakpoint

modular/gater/router.go

store/sqldb/migrate.go

base/gfspapp/manage_server.go

modular/gater/bucket_handler.go

modular/gater/migrate_handler.go

modular/metadata/metadata_sp_exit_service.go

alexgao001 · 2023-12-15T10:18:14Z

modular/executor/execute_gc.go

-				err = e.gcWorker.deleteObjectSegmentsAndIntegrity(ctx, objectInfo)
-				log.CtxInfow(ctx, "succeed to delete objects by gvg and bucket for gc", "object", objectInfo, "error", err)
+		for {
+			if objectList, err = e.baseApp.GfSpClient().ListObjectsByGVGAndBucketForGC(ctx, gvg.GetId(), bucketID, startAfter, limit); err != nil {


not find out startAfter is updated in this for loop.

alexgao001 · 2023-12-15T10:25:49Z

modular/gater/migrate_handler.go

 		log.CtxErrorw(reqCtx.Context(), "failed to pre migrate bucket, the bucket may already notified", "bucket_id",
 			bucketID, "error", err)
+		// if the bucket has already pre notified ignore the error
+		if strings.Contains(err.Error(), "the bucket has already notified") {


the err can be nil here?

yes, if the quota has been deducted, the preMigrateBucket should be return true;

alexgao001 · 2023-12-15T10:36:45Z

modular/manager/bucket_migrate_scheduler.go

@@ -526,32 +555,68 @@ func (s *BucketMigrateScheduler) doneMigrateBucket(bucketID uint64) error {
 	s.deleteExecutePlanByBucketID(bucketID)
 	executePlan.stopSPSchedule()
 	err = s.manager.baseApp.GfSpDB().DeleteMigrateGVGUnitsByBucketID(bucketID)


would this records stay in DB if skipping?

alexgao001 · 2023-12-15T10:39:51Z

modular/manager/bucket_migrate_scheduler.go

+		if err = UpdateBucketMigrationProgress(s.manager.baseApp, bucketID, storetypes.BucketMigrationState_MIGRATION_FINISHED); err != nil {
+			return
+		}
+		log.CtxInfow(ctx, "succeed to confirm complete events", "EventMigrationBucket", event, "error", err)


alexgao001 · 2023-12-15T10:46:36Z

modular/manager/migrate_service.go

+	// get bucket quota and check
+	if bucketSize, err = m.getBucketTotalSize(ctx, bucketID); err != nil {
+		log.Errorf("failed to get bucket total object size", "bucket_id", bucketID, "error", err)
+		return latestQuota, err
 	}

 	if bmInfo.GetFinished() {


seems this useless if

deleted, this is the old bucket migration gc trigger way.

will-2012 · 2023-12-18T05:28:10Z

base/gfspclient/gater.go

@@ -243,7 +245,7 @@ func (s *GfSpClient) QueryLatestBucketQuota(ctx context.Context, endpoint string
 		return gfsptask.GfSpBucketQuotaInfo{}, fmt.Errorf("failed to query latest bucket quota, bucket(%s), status_code(%d), endpoint(%s)", queryMsg, resp.StatusCode, endpoint)
 	}

-	signedMsg, err := hex.DecodeString(resp.Header.Get(GnfdSignedApprovalMsgHeader))
+	signedMsg, err := hex.DecodeString(resp.Header.Get(GnfdQuotaInfoHeader))


signedMsg may be misleading, it can be optimized.

will-2012 · 2023-12-18T05:30:00Z

core/spdb/entity.go

+type MigrateBucketProgressMeta struct {
+	BucketID              uint64 // as primary key
+	SubscribedBlockHeight uint64
+	MigrationState        int


migrateState?

jingjunLi requested review from ruojunm, will-2012, unclezoro and KeefeL as code owners November 15, 2023 13:20

jingjunLi added the wip Working in process label Nov 16, 2023

jingjunLi force-pushed the feat-add-bucket-status branch from 41bd8e5 to 9e915e4 Compare November 16, 2023 02:26

jingjunLi changed the title ~~feat: add bucket status~~ feat: pre & post MigrateBucketHandler persist improvement Nov 17, 2023

jingjunLi added r4r Ready for review wip Working in process and removed wip Working in process r4r Ready for review labels Nov 17, 2023

jingjunLi force-pushed the feat-add-bucket-status branch from 9e915e4 to 2decd3c Compare November 27, 2023 08:44

jingjunLi force-pushed the feat-add-bucket-status branch 2 times, most recently from 8560aa1 to 29c40d6 Compare December 6, 2023 06:06

jingjunLi changed the title ~~feat: pre & post MigrateBucketHandler persist improvement~~ feat: bucket migration state improvement Dec 6, 2023

jingjunLi added r4r Ready for review and removed wip Working in process labels Dec 6, 2023

jingjunLi assigned RenRick and sysvm and unassigned RenRick and sysvm Dec 6, 2023

jingjunLi requested review from RenRick and sysvm December 6, 2023 06:39

jingjunLi force-pushed the feat-add-bucket-status branch 2 times, most recently from 8a68460 to 578d0e0 Compare December 11, 2023 05:20

jingjunLi added 5 commits December 11, 2023 13:31

feat: add bucket migrate status persist & confirm complete event

ca7a1ac

feat: bucket migration gc report

7b66ebf

feat: add bucket migration gc progress & migration state persist

4d5c8d8

feat: sp gc state persist finished

50dabe5

feat: support bucket migration progress

a05ebb4

feat: fix some comment

f4b398d

sysvm approved these changes Dec 13, 2023

View reviewed changes

will-2012 reviewed Dec 13, 2023

View reviewed changes

store/sqldb/migrate.go Outdated Show resolved Hide resolved

store/sqldb/migrate.go Outdated Show resolved Hide resolved

store/sqldb/migrate.go Outdated Show resolved Hide resolved

jingjunLi added 7 commits December 14, 2023 09:32

feat: fix some variable name as comment

cc2df52

feat: fix bucket migration progress

88f6f6d

feat: rename some variable name

b144508

feat: rename some variable name

1d34ec7

feat: empty bucket quota sync

532578a

feat: fix some bugs for migrate

55d71ae

feat: fix some variable name

a30c03d

ruojunm reviewed Dec 15, 2023

View reviewed changes

base/gfspapp/manage_server.go Show resolved Hide resolved

modular/gater/bucket_handler.go Show resolved Hide resolved

modular/gater/migrate_handler.go Show resolved Hide resolved

modular/metadata/metadata_sp_exit_service.go Show resolved Hide resolved

ruojunm approved these changes Dec 15, 2023

View reviewed changes

jingjunLi added 5 commits December 15, 2023 15:22

feat: add pre & post ut

c0ab6e4

feat: add more ut

9c19f50

feat: format code

ba2951e

feat: format code

1b48287

feat: reject migrate bucket when current migrate not finished

25e5926

alexgao001 reviewed Dec 15, 2023

View reviewed changes

jingjunLi added 3 commits December 18, 2023 08:25

feat: add load from db for bucket migration gc task

e4593f2

feat: proto buf lint

1199274

feat: fix query bucket info from chain

aa08db2

alexgao001 approved these changes Dec 18, 2023

View reviewed changes

will-2012 approved these changes Dec 18, 2023

View reviewed changes

jingjunLi added 2 commits December 18, 2023 13:48

feat: rename some variable name

51207f8

feat: add config for enable bucket migrate cache

d5d24a2

jingjunLi merged commit e7aeec1 into develop Dec 18, 2023
12 checks passed

jingjunLi deleted the feat-add-bucket-status branch December 18, 2023 06:25

ruojunm mentioned this pull request Feb 21, 2024

docs: release for v1.4.0 #1349

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: bucket migration state improvement #1250

feat: bucket migration state improvement #1250

jingjunLi commented Nov 15, 2023 •

edited

sysvm left a comment

will-2012 Dec 13, 2023

will-2012 Dec 13, 2023

jingjunLi Dec 14, 2023

jingjunLi Dec 15, 2023

will-2012 Dec 13, 2023

will-2012 Dec 13, 2023

jingjunLi Dec 15, 2023

will-2012 Dec 13, 2023 •

edited

jingjunLi Dec 14, 2023

will-2012 Dec 13, 2023

jingjunLi Dec 15, 2023

alexgao001 Dec 15, 2023

jingjunLi Dec 18, 2023

alexgao001 Dec 15, 2023

jingjunLi Dec 18, 2023

alexgao001 Dec 15, 2023

alexgao001 Dec 15, 2023

jingjunLi Dec 18, 2023

alexgao001 Dec 15, 2023

jingjunLi Dec 18, 2023

will-2012 Dec 18, 2023

jingjunLi Dec 18, 2023

will-2012 Dec 18, 2023

jingjunLi Dec 18, 2023

		@@ -384,13 +384,16 @@ func (e *ExecuteModular) gcMetaReadRecord(ctx context.Context, task coretask.GCM
		}

		func (e *ExecuteModular) HandleGCBucketMigrationBucket(ctx context.Context, task coretask.GCBucketMigrationTask) {

feat: bucket migration state improvement #1250

feat: bucket migration state improvement #1250

Conversation

jingjunLi commented Nov 15, 2023 • edited

Description

Feat 1: Persistence of Quota State

feat 2: Optimization of state persistence in DB for each phase of bucket migration

feat 3: Bucket Migration Progress Query

Rationale

Example

Changes

Potential Impacts

sysvm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

will-2012 Dec 13, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jingjunLi commented Nov 15, 2023 •

edited

will-2012 Dec 13, 2023 •

edited