Skip to content

Commit

Permalink
chore: add lint errcheck and fix errcheck(dragonflyoss#766)
Browse files Browse the repository at this point in the history
* feat: add lint errcheck and fix errcheck

Signed-off-by: Gaius <[email protected]>

* replace assert to require

Signed-off-by: 孙伟鹏 <[email protected]>

Co-authored-by: 孙伟鹏 <[email protected]>
  • Loading branch information
gaius-qi and 244372610 authored Nov 15, 2021
1 parent d389ce2 commit 9dca7ee
Show file tree
Hide file tree
Showing 50 changed files with 415 additions and 211 deletions.
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ linters:
- deadcode
- gocyclo
- staticcheck
- errcheck

output:
format: colored-line-number
Expand Down
49 changes: 29 additions & 20 deletions cdn/plugins/plugins_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,19 @@ func (s *PluginsTestSuite) TestPluginBuilder() {
manager := NewManager()

var testFunc = func(pt PluginType, name string, b Builder, result bool) {
manager.AddBuilder(pt, name, b)
obj, _ := manager.GetBuilder(pt, name)
if result {
s.NotNil(obj)
err := manager.AddBuilder(pt, name, b)
if !result {
s.Require().NotNil(err)
}
obj, ok := manager.GetBuilder(pt, name)
if ok {
s.Require().NotNil(obj)
objVal := reflect.ValueOf(obj)
bVal := reflect.ValueOf(b)
s.Equal(objVal.Pointer(), bVal.Pointer())
s.Require().Equal(objVal.Pointer(), bVal.Pointer())
manager.DeleteBuilder(pt, name)
} else {
s.Nil(obj)
s.Require().Nil(obj)
}
}

Expand All @@ -76,14 +79,17 @@ func (s *PluginsTestSuite) TestManagerPlugin() {
manager := NewManager()

var testFunc = func(p Plugin, result bool) {
manager.AddPlugin(p)
obj, _ := manager.GetPlugin(p.Type(), p.Name())
if result {
s.NotNil(obj)
s.Equal(obj, p)
err := manager.AddPlugin(p)
if !result {
s.Require().NotNil(err)
}
obj, ok := manager.GetPlugin(p.Type(), p.Name())
if ok {
s.Require().NotNil(obj)
s.Require().Equal(obj, p)
manager.DeletePlugin(p.Type(), p.Name())
} else {
s.Nil(obj)
s.Require().Nil(obj)
}
}

Expand Down Expand Up @@ -128,16 +134,19 @@ func (s *PluginsTestSuite) TestRepositoryIml() {

repo := NewRepository()
for _, v := range cases {
repo.Add(v.pt, v.name, v.data)
data, _ := repo.Get(v.pt, v.name)
if v.addResult {
s.NotNil(data)
s.Equal(data, v.data)
err := repo.Add(v.pt, v.name, v.data)
if !v.addResult {
s.Require().NotNil(err)
}
data, ok := repo.Get(v.pt, v.name)
if ok {
s.Require().NotNil(data)
s.Require().Equal(data, v.data)
repo.Delete(v.pt, v.name)
data, _ = repo.Get(v.pt, v.name)
s.Nil(data)
s.Require().Nil(data)
} else {
s.Nil(data)
s.Require().Nil(data)
}
}
}
Expand All @@ -159,7 +168,7 @@ func (s *PluginsTestSuite) TestValidate() {
)
}
for _, v := range cases {
s.Equal(validate(v.pt, v.name), v.expected)
s.Require().Equal(validate(v.pt, v.name), v.expected)
}
}

Expand Down
9 changes: 7 additions & 2 deletions cdn/storedriver/local/local_driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,13 @@ const (
var fileLocker = synclock.NewLockerPool()

func init() {
storedriver.Register(DiskDriverName, NewStorageDriver)
storedriver.Register(MemoryDriverName, NewStorageDriver)
if err := storedriver.Register(DiskDriverName, NewStorageDriver); err != nil {
logger.CoreLogger.Error(err)
}

if err := storedriver.Register(MemoryDriverName, NewStorageDriver); err != nil {
logger.CoreLogger.Error(err)
}
}

// driver is one of the implementations of storage Driver using local file system.
Expand Down
17 changes: 13 additions & 4 deletions cdn/supervisor/cdn/storage/disk/disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ var (
)

func init() {
storage.Register(StorageMode, newStorageManager)
if err := storage.Register(StorageMode, newStorageManager); err != nil {
logger.CoreLogger.Error(err)
}
}

func newStorageManager(cfg *storage.Config) (storage.Manager, error) {
Expand Down Expand Up @@ -265,13 +267,20 @@ func (s *diskStorageMgr) TryFreeSpace(fileLength int64) (bool, error) {
return nil
},
}
s.diskDriver.Walk(r)
if err := s.diskDriver.Walk(r); err != nil {
return false, err
}

enoughSpace := freeSpace.ToNumber()-remainder.Load() > fileLength
if !enoughSpace {
s.cleaner.GC("disk", true)
if _, err := s.cleaner.GC("disk", true); err != nil {
return false, err
}

remainder.Store(0)
s.diskDriver.Walk(r)
if err := s.diskDriver.Walk(r); err != nil {
return false, err
}
freeSpace, err = s.diskDriver.GetFreeSpace()
if err != nil {
return false, err
Expand Down
29 changes: 22 additions & 7 deletions cdn/supervisor/cdn/storage/hybrid/hybrid.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ var _ storage.Manager = (*hybridStorageMgr)(nil)
var _ gc.Executor = (*hybridStorageMgr)(nil)

func init() {
storage.Register(StorageMode, newStorageManager)
if err := storage.Register(StorageMode, newStorageManager); err != nil {
logger.CoreLogger.Error(err)
}
}

// NewStorageManager performs initialization for storage manager and return a storage Manager.
Expand Down Expand Up @@ -325,13 +327,20 @@ func (h *hybridStorageMgr) TryFreeSpace(fileLength int64) (bool, error) {
return nil
},
}
h.diskDriver.Walk(r)
if err := h.diskDriver.Walk(r); err != nil {
return false, err
}

enoughSpace := diskFreeSpace.ToNumber()-remainder.Load() > fileLength
if !enoughSpace {
h.diskDriverCleaner.GC("hybrid", true)
if _, err := h.diskDriverCleaner.GC("hybrid", true); err != nil {
return false, err
}

remainder.Store(0)
h.diskDriver.Walk(r)
if err := h.diskDriver.Walk(r); err != nil {
return false, err
}
diskFreeSpace, err = h.diskDriver.GetFreeSpace()
if err != nil {
return false, err
Expand Down Expand Up @@ -397,7 +406,7 @@ func (h *hybridStorageMgr) deleteTaskFiles(taskID string, deleteUploadPath bool,
func (h *hybridStorageMgr) tryShmSpace(url, taskID string, fileLength int64) (string, error) {
if h.shmSwitch.check(url, fileLength) && h.hasShm {
remainder := atomic.NewInt64(0)
h.memoryDriver.Walk(&storedriver.Raw{
if err := h.memoryDriver.Walk(&storedriver.Raw{
WalkFn: func(filePath string, info os.FileInfo, err error) error {
if fileutils.IsRegular(filePath) {
taskID := strings.Split(path.Base(filePath), ".")[0]
Expand All @@ -416,12 +425,18 @@ func (h *hybridStorageMgr) tryShmSpace(url, taskID string, fileLength int64) (st
}
return nil
},
})
}); err != nil {
return "", err
}

canUseShm := h.getMemoryUsableSpace()-unit.Bytes(remainder.Load())-secureLevel >= unit.Bytes(
fileLength)
if !canUseShm {
// 如果剩余空间过小,则强制执行一次fullgc后在检查是否满足
h.memoryDriverCleaner.GC("hybrid", true)
if _, err := h.memoryDriverCleaner.GC("hybrid", true); err != nil {
return "", err
}

canUseShm = h.getMemoryUsableSpace()-unit.Bytes(remainder.Load())-secureLevel >= unit.Bytes(
fileLength)
}
Expand Down
9 changes: 7 additions & 2 deletions cdn/supervisor/task/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,9 @@ func (tm Manager) Delete(taskID string) error {
tm.accessTimeMap.Delete(taskID)
tm.taskURLUnReachableStore.Delete(taskID)
tm.taskStore.Delete(taskID)
tm.progressMgr.Clear(taskID)
if err := tm.progressMgr.Clear(taskID); err != nil {
return err
}
return nil
}

Expand Down Expand Up @@ -227,7 +229,10 @@ func (tm *Manager) GC() error {
}
// gc task memory data
logger.GcLogger.With("type", "meta").Infof("gc task: start to deal with task: %s", taskID)
tm.Delete(taskID)
if err := tm.Delete(taskID); err != nil {
logger.GcLogger.With("type", "meta").Infof("gc task: failed to delete task: %s", taskID)
continue
}
removedTaskCount++
}

Expand Down
10 changes: 7 additions & 3 deletions cdn/supervisor/task/manager_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ func (tm *Manager) addOrUpdateTask(ctx context.Context, request *types.TaskRegis
if err != nil {
task.Log().Errorf("failed to get url (%s) content length: %v", task.URL, err)
if cdnerrors.IsURLNotReachable(err) {
tm.taskURLUnReachableStore.Add(taskID, time.Now())
if err := tm.taskURLUnReachableStore.Add(taskID, time.Now()); err != nil {
task.Log().Errorf("failed to add url (%s) to unreachable store: %v", task.URL, err)
}
return nil, err
}
}
Expand All @@ -119,9 +121,11 @@ func (tm *Manager) addOrUpdateTask(ctx context.Context, request *types.TaskRegis
pieceSize := cdnutil.ComputePieceSize(task.SourceFileLength)
task.PieceSize = pieceSize
}
tm.taskStore.Add(task.TaskID, task)
logger.Debugf("success add task: %+v into taskStore", task)
if err := tm.taskStore.Add(task.TaskID, task); err != nil {
return nil, err
}

logger.Debugf("success add task: %+v into taskStore", task)
return task, nil
}

Expand Down
8 changes: 6 additions & 2 deletions client/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -441,10 +441,14 @@ func (cd *clientDaemon) Stop() {
close(cd.done)
cd.GCManager.Stop()
cd.RPCManager.Stop()
cd.UploadManager.Stop()
if err := cd.UploadManager.Stop(); err != nil {
logger.Errorf("upload manager stop failed %s", err)
}

if cd.ProxyManager.IsEnabled() {
cd.ProxyManager.Stop()
if err := cd.ProxyManager.Stop(); err != nil {
logger.Errorf("proxy manager stop failed %s", err)
}
}

if !cd.Option.KeepStorage {
Expand Down
15 changes: 11 additions & 4 deletions client/daemon/peer/peertask_base.go
Original file line number Diff line number Diff line change
Expand Up @@ -415,9 +415,13 @@ loop:
if pt.failedCode == failedCodeNotSet {
pt.failedReason = reasonContextCanceled
pt.failedCode = dfcodes.ClientContextCanceled
pt.callback.Fail(pt, pt.failedCode, pt.ctx.Err().Error())
if err := pt.callback.Fail(pt, pt.failedCode, pt.ctx.Err().Error()); err != nil {
pt.Errorf("peer task callback failed %s", err)
}
} else {
pt.callback.Fail(pt, pt.failedCode, pt.failedReason)
if err := pt.callback.Fail(pt, pt.failedCode, pt.failedReason); err != nil {
pt.Errorf("peer task callback failed %s", err)
}
}
}
break loop
Expand Down Expand Up @@ -661,7 +665,7 @@ func (pt *peerTask) downloadPieceWorker(id int32, pti Task, requests chan *Downl
pt.Errorf("request limiter error: %s", err)
waitSpan.RecordError(err)
waitSpan.End()
pti.ReportPieceResult(&pieceTaskResult{
if err := pti.ReportPieceResult(&pieceTaskResult{
piece: request.piece,
pieceResult: &scheduler.PieceResult{
TaskId: pt.GetTaskID(),
Expand All @@ -674,7 +678,10 @@ func (pt *peerTask) downloadPieceWorker(id int32, pti Task, requests chan *Downl
FinishedCount: 0, // update by peer task
},
err: err,
})
}); err != nil {
pt.Errorf("report piece result failed %s", err)
}

pt.failedReason = err.Error()
pt.failedCode = dfcodes.ClientRequestLimitFail
pt.cancel()
Expand Down
8 changes: 7 additions & 1 deletion client/daemon/peer/peertask_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,13 @@ func setupPeerTaskManagerComponents(ctrl *gomock.Controller, opt componentsOptio
Type: "tcp",
Addr: fmt.Sprintf("0.0.0.0:%d", port),
})
go daemonserver.New(daemon).Serve(ln)

go func() {
if err := daemonserver.New(daemon).Serve(ln); err != nil {
panic(err)
}
}()

time.Sleep(100 * time.Millisecond)

// 2. setup a scheduler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ import (
"fmt"
"io"
"io/ioutil"
"log"
"math"
"net"
"sync"
"testing"
"time"
Expand Down Expand Up @@ -94,7 +96,11 @@ func setupBackSourcePartialComponents(ctrl *gomock.Controller, testBytes []byte,
Type: "tcp",
Addr: fmt.Sprintf("0.0.0.0:%d", port),
})
go daemonserver.New(daemon).Serve(ln)
go func(daemon *mock_daemon.MockDaemonServer, ln net.Listener) {
if err := daemonserver.New(daemon).Serve(ln); err != nil {
log.Fatal(err)
}
}(daemon, ln)
time.Sleep(100 * time.Millisecond)

// 2. setup a scheduler
Expand Down
16 changes: 12 additions & 4 deletions client/daemon/peer/piece_downloader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ func TestPieceDownloader_DownloadPiece(t *testing.T) {
assert.Equal(upload.PeerDownloadHTTPPathPrefix+"tas/"+"task-0", r.URL.Path)
data := []byte("test test ")
w.Header().Set(headers.ContentLength, fmt.Sprintf("%d", len(data)))
w.Write(data)
if _, err := w.Write(data); err != nil {
t.Error(err)
}
},
taskID: "task-0",
pieceRange: "bytes=0-9",
Expand All @@ -72,7 +74,9 @@ func TestPieceDownloader_DownloadPiece(t *testing.T) {
assert.Equal(upload.PeerDownloadHTTPPathPrefix+"tas/"+"task-1", r.URL.Path)
rg := clientutil.MustParseRange(r.Header.Get("Range"), math.MaxInt64)
w.Header().Set(headers.ContentLength, fmt.Sprintf("%d", rg.Length))
w.Write(testData[rg.Start : rg.Start+rg.Length])
if _, err := w.Write(testData[rg.Start : rg.Start+rg.Length]); err != nil {
t.Error(err)
}
},
taskID: "task-1",
pieceRange: "bytes=0-99",
Expand All @@ -85,7 +89,9 @@ func TestPieceDownloader_DownloadPiece(t *testing.T) {
assert.Equal(upload.PeerDownloadHTTPPathPrefix+"tas/"+"task-2", r.URL.Path)
rg := clientutil.MustParseRange(r.Header.Get("Range"), math.MaxInt64)
w.Header().Set(headers.ContentLength, fmt.Sprintf("%d", rg.Length))
w.Write(testData[rg.Start : rg.Start+rg.Length])
if _, err := w.Write(testData[rg.Start : rg.Start+rg.Length]); err != nil {
t.Error(err)
}
},
taskID: "task-2",
pieceRange: fmt.Sprintf("bytes=512-%d", len(testData)-1),
Expand All @@ -98,7 +104,9 @@ func TestPieceDownloader_DownloadPiece(t *testing.T) {
assert.Equal(upload.PeerDownloadHTTPPathPrefix+"tas/"+"task-3", r.URL.Path)
rg := clientutil.MustParseRange(r.Header.Get("Range"), math.MaxInt64)
w.Header().Set(headers.ContentLength, fmt.Sprintf("%d", rg.Length))
w.Write(testData[rg.Start : rg.Start+rg.Length])
if _, err := w.Write(testData[rg.Start : rg.Start+rg.Length]); err != nil {
t.Error(err)
}
},
taskID: "task-3",
pieceRange: "bytes=512-1024",
Expand Down
Loading

0 comments on commit 9dca7ee

Please sign in to comment.