From 243bf1a0b3f320f55d5657d6c9a386c254504887 Mon Sep 17 00:00:00 2001 From: LexLuthr Date: Thu, 4 Apr 2024 17:42:21 +0400 Subject: [PATCH] refactor MinerFilter func, update comments --- .../sql/20240401-storage-miner-filter.sql | 2 + .../sql/20240401-storage-segregation.sql | 1 - storage/paths/db_index.go | 29 ++------ storage/paths/index.go | 69 +++++++------------ storage/paths/local.go | 9 +-- storage/sealer/storiface/filetype.go | 64 +++++++---------- 6 files changed, 56 insertions(+), 118 deletions(-) create mode 100644 lib/harmony/harmonydb/sql/20240401-storage-miner-filter.sql delete mode 100644 lib/harmony/harmonydb/sql/20240401-storage-segregation.sql diff --git a/lib/harmony/harmonydb/sql/20240401-storage-miner-filter.sql b/lib/harmony/harmonydb/sql/20240401-storage-miner-filter.sql new file mode 100644 index 000000000..27cecafe6 --- /dev/null +++ b/lib/harmony/harmonydb/sql/20240401-storage-miner-filter.sql @@ -0,0 +1,2 @@ +ALTER TABLE storage_path ADD COLUMN IF NOT EXISTS allow_miners varchar DEFAULT ''; +ALTER TABLE storage_path ADD COLUMN IF NOT EXISTS deny_miners varchar DEFAULT ''; -- comma separated list of miner addresses \ No newline at end of file diff --git a/lib/harmony/harmonydb/sql/20240401-storage-segregation.sql b/lib/harmony/harmonydb/sql/20240401-storage-segregation.sql deleted file mode 100644 index bcc29e68c..000000000 --- a/lib/harmony/harmonydb/sql/20240401-storage-segregation.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER TABLE storage_path ADD COLUMN IF NOT EXISTS allow_miners varchar, ADD COLUMN IF NOT EXISTS deny_miners varchar; -- comma separated list of miner addresses \ No newline at end of file diff --git a/storage/paths/db_index.go b/storage/paths/db_index.go index daebc3607..948398e33 100644 --- a/storage/paths/db_index.go +++ b/storage/paths/db_index.go @@ -24,6 +24,8 @@ import ( "github.com/filecoin-project/lotus/storage/sealer/storiface" ) +const NoMinerFilter = abi.ActorID(0) + var errAlreadyLocked = errors.New("already locked") type DBIndex struct { @@ -578,21 +580,13 @@ func (dbi *DBIndex) StorageFindSector(ctx context.Context, s abi.SectorID, ft st continue } allowMiners := splitString(row.AllowMiners) - proceed, err := MinerFilter(allowMiners, false, s.Miner) - if err != nil { - return nil, err - } - if !proceed { - log.Debugf("not allocating on %s, miner %s not allowed", row.StorageId, s.Miner.String()) - continue - } denyMiners := splitString(row.DenyMiners) - proceed, err = MinerFilter(denyMiners, true, s.Miner) + proceed, msg, err := MinerFilter(allowMiners, denyMiners, s.Miner) if err != nil { return nil, err } if !proceed { - log.Debugf("not allocating on %s, miner %s denied", row.StorageId, s.Miner.String()) + log.Debugf("not allocating on %s, miner %s %s", row.StorageId, s.Miner.String(), msg) continue } @@ -734,7 +728,6 @@ func (dbi *DBIndex) StorageBestAlloc(ctx context.Context, allocate storiface.Sec SkippedHeartbeatThresh.Seconds(), pathType == storiface.PathSealing, pathType == storiface.PathStorage, - miner.String(), ) if err != nil { return nil, xerrors.Errorf("Querying for best storage sectors fails with err %w: ", err) @@ -745,23 +738,15 @@ func (dbi *DBIndex) StorageBestAlloc(ctx context.Context, allocate storiface.Sec for _, row := range rows { // Matching with 0 as a workaround to avoid having minerID // present when calling TaskStorage.HasCapacity() - if !(miner == abi.ActorID(0)) { + if !(miner == NoMinerFilter) { allowMiners := splitString(row.AllowMiners) - proceed, err := MinerFilter(allowMiners, false, miner) - if err != nil { - return nil, err - } - if !proceed { - log.Debugf("not allocating on %s, miner %s not allowed", row.StorageId, miner.String()) - continue - } denyMiners := splitString(row.DenyMiners) - proceed, err = MinerFilter(denyMiners, true, miner) + proceed, msg, err := MinerFilter(allowMiners, denyMiners, miner) if err != nil { return nil, err } if !proceed { - log.Debugf("not allocating on %s, miner %s denied", row.StorageId, miner.String()) + log.Debugf("not allocating on %s, miner %s %s", row.StorageId, miner.String(), msg) continue } } diff --git a/storage/paths/index.go b/storage/paths/index.go index 391e42972..e09b887e0 100644 --- a/storage/paths/index.go +++ b/storage/paths/index.go @@ -498,23 +498,13 @@ func (i *MemIndex) StorageFindSector(ctx context.Context, s abi.SectorID, ft sto continue } - proceed, err := MinerFilter(st.info.AllowMiners, false, s.Miner) + proceed, msg, err := MinerFilter(st.info.AllowMiners, st.info.DenyMiners, s.Miner) if err != nil { return nil, err } if !proceed { - log.Debugf("not allocating on %s, miner %s not allowed", st.info.ID, s.Miner.String()) - continue - } - - proceed, err = MinerFilter(st.info.DenyMiners, true, s.Miner) - if err != nil { - return nil, err - } - - if !proceed { - log.Debugf("not allocating on %s, miner %s denied", st.info.ID, s.Miner.String()) + log.Debugf("not allocating on %s, miner %s %s", st.info.ID, s.Miner.String(), msg) continue } @@ -673,23 +663,13 @@ func (i *MemIndex) StorageBestAlloc(ctx context.Context, allocate storiface.Sect continue } - proceed, err := MinerFilter(p.info.AllowMiners, false, miner) + proceed, msg, err := MinerFilter(p.info.AllowMiners, p.info.DenyMiners, miner) if err != nil { return nil, err } if !proceed { - log.Debugf("not allocating on %s, miner %s not allowed", p.info.ID, miner.String()) - continue - } - - proceed, err = MinerFilter(p.info.DenyMiners, true, miner) - if err != nil { - return nil, err - } - - if !proceed { - log.Debugf("not allocating on %s, miner %s denied", p.info.ID, miner.String()) + log.Debugf("not allocating on %s, miner %s %s", p.info.ID, miner.String(), msg) continue } @@ -751,10 +731,9 @@ func (i *MemIndex) FindSector(id abi.SectorID, typ storiface.SectorFileType) ([] var _ SectorIndex = &MemIndex{} -func MinerFilter(miners []string, deny bool, miner abi.ActorID) (bool, error) { - if len(miners) > 0 { - found := false - for _, m := range miners { +func MinerFilter(allowMiners, denyMiners []string, miner abi.ActorID) (bool, string, error) { + checkMinerInList := func(minersList []string, miner abi.ActorID) (bool, error) { + for _, m := range minersList { minerIDStr := m maddr, err := address.NewFromString(minerIDStr) if err != nil { @@ -765,26 +744,24 @@ func MinerFilter(miners []string, deny bool, miner abi.ActorID) (bool, error) { return false, xerrors.Errorf("converting miner address to ID: %w", err) } if abi.ActorID(mid) == miner { - found = true - break + return true, nil } } - // If not found in list - if !found { - // If this is allowed list - if !deny { - return false, nil - } - // If this is denied list - return true, nil - } - // If found in list and - // If this is allowed list - if !deny { - return true, nil - } - // If this is denied list return false, nil } - return true, nil + + if len(allowMiners) > 0 { + found, err := checkMinerInList(allowMiners, miner) + if err != nil || !found { + return false, "not allowed", err + } + } + + if len(denyMiners) > 0 { + found, err := checkMinerInList(denyMiners, miner) + if err != nil || found { + return false, "denied", err + } + } + return true, "", nil } diff --git a/storage/paths/local.go b/storage/paths/local.go index 018537c82..340e4f279 100644 --- a/storage/paths/local.go +++ b/storage/paths/local.go @@ -511,14 +511,7 @@ func (st *Local) AcquireSector(ctx context.Context, sid storiface.SectorRef, exi if !fileType.Allowed(allowTypes, denyTypes) { return false, nil } - proceed, err := MinerFilter(allowMiners, false, miner) - if err != nil { - return false, err - } - if !proceed { - return false, nil - } - proceed, err = MinerFilter(denyMiners, true, miner) + proceed, _, err := MinerFilter(allowMiners, denyMiners, miner) if err != nil { return false, err } diff --git a/storage/sealer/storiface/filetype.go b/storage/sealer/storiface/filetype.go index dc8bebc7e..0aa28861b 100644 --- a/storage/sealer/storiface/filetype.go +++ b/storage/sealer/storiface/filetype.go @@ -8,28 +8,28 @@ import ( "github.com/filecoin-project/go-state-types/abi" ) -// FTUnsealed represents an unsealed sector file type. -// FTSealed represents a sealed sector file type. -// FTCache represents a cache sector file type. -// FTUpdate represents an update sector file type. -// FTUpdateCache represents an update cache sector file type. -// FTPiece represents a Piece Park sector file type. -// FileTypes represents the total number of file types. +// FTUnsealed represents an unsealed sector file. +// FTSealed represents a sealed sector file. +// FTCache represents a cache sector file. +// FTUpdate represents an update sector file. +// FTUpdateCache represents an update cache sector file. +// FTPiece represents a Piece Park sector file. +// FileTypes represents the total number of file. // -// The SectorFileType type is an integer type that represents different sector file types. -// It has several methods to manipulate and query the file type. -// The String method returns a string representation of the file type. -// The Strings method returns a slice of string representations of all the file types that are set in the receiver object. -// The AllSet method returns a slice of all the file types that are set in the receiver object. +// The SectorFileType type is an integer type that represents different sector file. +// It has several methods to manipulate and query the file. +// The String method returns a string representation of the file. +// The Strings method returns a slice of string representations of all the file that are set in the receiver object. +// The AllSet method returns a slice of all the file that are set in the receiver object. // The Has method checks whether a specific file type is set in the receiver object. // The SealSpaceUse method calculates the space used by the receiver object in sealing a sector of a given size. -// The SubAllowed method removes selected file types from the receiver object based on a list of allowed and denied file types. -// The Unset method removes selected file types from the receiver object. -// The AnyAllowed method checks whether any file types in the receiver object are allowed based on a list of allowed and denied file types. -// The Allowed method checks whether all file types in the receiver object are allowed based on a list of allowed and denied file types. +// The SubAllowed method removes selected file from the receiver object based on a list of allowed and denied file. +// The Unset method removes selected file from the receiver object. +// The AnyAllowed method checks whether any file in the receiver object are allowed based on a list of allowed and denied file. +// The Allowed method checks whether all file in the receiver object are allowed based on a list of allowed and denied file. // The StoreSpaceUse method calculates the space used by the receiver object in storing a sector of a given size. -// The All method returns an array that represents which file types are set in the receiver object. -// The IsNone method checks whether the receiver object represents no file types. +// The All method returns an array that represents which file are set in the receiver object. +// The IsNone method checks whether the receiver object represents no file. const ( // "regular" sectors FTUnsealed SectorFileType = 1 << iota @@ -56,14 +56,14 @@ const ( // - FTPiece: represents Piece Park sectors var PathTypes = []SectorFileType{FTUnsealed, FTSealed, FTCache, FTUpdate, FTUpdateCache, FTPiece} -// FTNone represents a sector file type of none. This constant is used in the StorageLock method to specify that a sector should not have any file types locked. +// FTNone represents a sector file type of none. This constant is used in the StorageLock method to specify that a sector should not have any file locked. // Example usage: // err := m.index.StorageLock(ctx, sector.ID, storiface.FTNone, storiface.FTSealed|storiface.FTUnsealed|storiface.FTCache) const ( FTNone SectorFileType = 0 ) -// FTAll represents the combination of all available sector file types. +// FTAll represents the combination of all available sector file. // It is a variable of type SectorFileType. // The value of FTAll is calculated by iterating over the PathTypes slice and using the |= operator to perform a bitwise OR operation on each path type. // The result is assigned to the variable out and returned. @@ -78,7 +78,7 @@ var FTAll = func() (out SectorFileType) { // FSOverheadDen represents the constant value 10, which is used to calculate the overhead in various storage space utilization calculations. const FSOverheadDen = 10 -// FSOverheadSeal is a map that represents the overheads for different SectorFileType in sealed sectors. +// FSOverheadSeal is a map that represents the overheads for different SectorFileType in sectors which are being sealed. var FSOverheadSeal = map[SectorFileType]int{ // 10x overheads FTUnsealed: FSOverheadDen, FTSealed: FSOverheadDen, @@ -93,27 +93,9 @@ var FSOverheadSeal = map[SectorFileType]int{ // 10x overheads // FsOverheadFinalized is a map that represents the finalized overhead for different types of SectorFileType. // The keys in the map are the SectorFileType values, and the values are integers representing the overhead. // It is used to calculate the storage space usage for different types of sectors, as shown in the example below: -// -// func (t SectorFileType) StoreSpaceUse(ssize abi.SectorSize) (uint64, error) { -// var need uint64 -// for _, pathType := range PathTypes { -// if !t.Has(pathType) { -// continue -// } -// -// oh, ok := FsOverheadFinalized[pathType] -// if !ok { -// return 0, xerrors.Errorf("no finalized overhead info for %s", pathType) -// } -// -// need += uint64(oh) * uint64(ssize) / FSOverheadDen -// } -// -// return need, nil -// } -// // The overhead value is retrieved from FsOverheadFinalized by using the SectorFileType value as the key. -// If the overhead value is not found in the map, an error is returned indicating that there is no finalized overhead information for the given sector type. +// If the overhead value is not found in the map, an error is returned indicating that there is no finalized +// overhead information for the given sector type. var FsOverheadFinalized = map[SectorFileType]int{ FTUnsealed: FSOverheadDen, FTSealed: FSOverheadDen,