perf: slow down DHT announcing on errors (#1186)

pkg/download/downloader_test.go

@@ -757,7 +757,7 @@ func TestDownloader_Delete(t *testing.T) {
 	})
 }
 
-func TestDownloader_ContinueBatch(t *testing.T) {
+func TestDownloader_PauseAndContinue(t *testing.T) {
 	listener := test.StartTestSlowFileServer(time.Millisecond * 2000)
 	defer listener.Close()
 
@@ -767,7 +767,74 @@ func TestDownloader_ContinueBatch(t *testing.T) {
 	}
 	defer downloader.Clear()
 
-	// Create multiple tasks and pause them
+	// Create a single task
+	req := &base.Request{
+		URL: "http://" + listener.Addr().String() + "/" + test.BuildName,
+	}
+	rr, err := downloader.Resolve(req, testDownloadOpt)
+	if err != nil {
+		t.Fatal(err)
+	}
+	taskId, err := downloader.Create(rr.ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Wait for task to start
+	time.Sleep(time.Millisecond * 100)
+
+	// Pause with specific taskId
+	err = downloader.Pause(&TaskFilter{IDs: []string{taskId}})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	time.Sleep(time.Millisecond * 100)
+
+	// Verify task is paused
+	task := downloader.GetTask(taskId)
+	if task.Status != base.DownloadStatusPause {
+		t.Errorf("Task should be paused, got %s", task.Status)
+	}
+
+	// Continue with specific taskId
+	var wg sync.WaitGroup
+	wg.Add(1)
+	downloader.Listener(func(event *Event) {
+		if event.Key == EventKeyDone {
+			wg.Done()
+		}
+	})
+
+	err = downloader.Continue(&TaskFilter{IDs: []string{taskId}})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Wait for task to complete
+	wg.Wait()
+
+	// Verify task is done
+	task = downloader.GetTask(taskId)
+	if task.Status != base.DownloadStatusDone {
+		t.Errorf("Task should be done, got %s", task.Status)
+	}
+
+	// Clean up
+	downloader.Delete(nil, true)
+}
+
+func TestDownloader_PauseAllAndContinueAll(t *testing.T) {
+	listener := test.StartTestSlowFileServer(time.Millisecond * 2000)
+	defer listener.Close()
+
+	downloader := NewDownloader(nil)
+	if err := downloader.Setup(); err != nil {
+		t.Fatal(err)
+	}
+	defer downloader.Clear()
+
+	// Create multiple tasks
 	taskCount := 3
 	taskIds := make([]string, 0)
 
@@ -786,183 +853,57 @@ func TestDownloader_ContinueBatch(t *testing.T) {
 		taskIds = append(taskIds, taskId)
 	}
 
-	// Wait a moment for tasks to start
-	time.Sleep(time.Millisecond * 500)
+	// Wait for tasks to start
+	time.Sleep(time.Millisecond * 100)
 
-	// Pause all tasks
+	// Pause all tasks with nil filter
 	err := downloader.Pause(nil)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	// Verify tasks are paused
+	time.Sleep(time.Millisecond * 100)
+
+	// Verify all tasks are paused
+	pausedCount := 0
 	for _, taskId := range taskIds {
 		task := downloader.GetTask(taskId)
-		if task.Status != base.DownloadStatusPause && task.Status != base.DownloadStatusDone {
-			t.Errorf("Task %s should be paused or done, got %s", taskId, task.Status)
+		if task.Status == base.DownloadStatusPause {
+			pausedCount++
 		}
 	}
+	if pausedCount != taskCount {
+		t.Errorf("Expected %d paused tasks, got %d", taskCount, pausedCount)
+	}
 
-	// Test ContinueBatch with specific IDs
-	t.Run("ContinueBatch with filter", func(t *testing.T) {
-		var wg sync.WaitGroup
-		wg.Add(1)
-		downloader.Listener(func(event *Event) {
-			if event.Key == EventKeyDone {
-				wg.Done()
-			}
-		})
-
-		err := downloader.ContinueBatch(&TaskFilter{IDs: []string{taskIds[0]}})
-		if err != nil {
-			t.Errorf("ContinueBatch() unexpected error: %v", err)
-		}
-		wg.Wait()
-	})
-
-	// Test ContinueBatch with nil filter (continues all)
-	t.Run("ContinueBatch with nil filter", func(t *testing.T) {
-		var wg sync.WaitGroup
-		// Count remaining paused tasks
-		pausedCount := 0
-		for _, taskId := range taskIds {
-			task := downloader.GetTask(taskId)
-			if task.Status == base.DownloadStatusPause {
-				pausedCount++
-			}
-		}
-		wg.Add(pausedCount)
-		downloader.Listener(func(event *Event) {
-			if event.Key == EventKeyDone {
-				wg.Done()
-			}
-		})
-
-		err := downloader.ContinueBatch(nil)
-		if err != nil {
-			t.Errorf("ContinueBatch() unexpected error: %v", err)
-		}
-		if pausedCount > 0 {
-			wg.Wait()
+	// Continue all tasks with nil filter
+	var wg sync.WaitGroup
+	wg.Add(taskCount)
+	downloader.Listener(func(event *Event) {
+		if event.Key == EventKeyDone {
+			wg.Done()
 		}
 	})
 
-	// Clean up
-	downloader.Delete(nil, true)
-}
-
-func TestDownloader_PauseAndContinue(t *testing.T) {
-	listener := test.StartTestSlowFileServer(time.Millisecond * 5000)
-	defer listener.Close()
-
-	downloader := NewDownloader(nil)
-	if err := downloader.Setup(); err != nil {
-		t.Fatal(err)
-	}
-	defer downloader.Clear()
-
-	req := &base.Request{
-		URL: "http://" + listener.Addr().String() + "/" + test.BuildName,
-	}
-	rr, err := downloader.Resolve(req, testDownloadOpt)
+	err = downloader.Continue(nil)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	taskId, err := downloader.Create(rr.ID)
-	if err != nil {
-		t.Fatal(err)
+	// Wait for all tasks to complete
+	wg.Wait()
+
+	// Verify all tasks are done
+	doneCount := 0
+	for _, taskId := range taskIds {
+		task := downloader.GetTask(taskId)
+		if task.Status == base.DownloadStatusDone {
+			doneCount++
+		}
+	}
+	if doneCount != taskCount {
+		t.Errorf("Expected %d done tasks, got %d", taskCount, doneCount)
 	}
-
-	// Wait for task to start
-	time.Sleep(time.Millisecond * 1000)
-
-	// Test Pause with empty filter (returns nil, not error - empty filter means no op)
-	t.Run("Pause with empty filter", func(t *testing.T) {
-		err := downloader.Pause(&TaskFilter{})
-		// Empty filter returns error from Pause function
-		if err == nil {
-			// This is fine - different implementations might handle this differently
-		}
-	})
-
-	// Test Pause with filter
-	t.Run("Pause with filter", func(t *testing.T) {
-		task := downloader.GetTask(taskId)
-		if task == nil {
-			t.Skip("Task already completed")
-		}
-		if task.Status == base.DownloadStatusDone {
-			t.Skip("Task already completed")
-		}
-
-		err := downloader.Pause(&TaskFilter{IDs: []string{taskId}})
-		if err != nil {
-			// Task might have finished between check and pause
-			t.Logf("Pause() error (task may have finished): %v", err)
-		} else {
-			task = downloader.GetTask(taskId)
-			if task.Status != base.DownloadStatusPause && task.Status != base.DownloadStatusDone {
-				t.Errorf("Task should be paused or done, got %s", task.Status)
-			}
-		}
-	})
-
-	// Test Pause with non-matching filter
-	t.Run("Pause with non-matching filter", func(t *testing.T) {
-		err := downloader.Pause(&TaskFilter{IDs: []string{"non-existent-id"}})
-		if err == nil {
-			t.Error("Pause() with non-matching filter should return error")
-		}
-	})
-
-	// Test Continue with empty filter (returns error - empty filter means no op)
-	t.Run("Continue with empty filter", func(t *testing.T) {
-		err := downloader.Continue(&TaskFilter{})
-		// Empty filter returns error from Continue function
-		if err == nil {
-			// This is fine - different implementations might handle this differently
-		}
-	})
-
-	// Test Continue with non-matching filter
-	t.Run("Continue with non-matching filter", func(t *testing.T) {
-		err := downloader.Continue(&TaskFilter{IDs: []string{"non-existent-id"}})
-		if err == nil {
-			t.Error("Continue() with non-matching filter should return error")
-		}
-	})
-
-	// Test Continue with valid filter
-	t.Run("Continue with valid filter", func(t *testing.T) {
-		task := downloader.GetTask(taskId)
-		if task == nil {
-			t.Skip("Task not found")
-		}
-		if task.Status == base.DownloadStatusDone {
-			t.Skip("Task already completed")
-		}
-
-		var wg sync.WaitGroup
-		wg.Add(1)
-		downloader.Listener(func(event *Event) {
-			if event.Key == EventKeyDone && event.Task != nil && event.Task.ID == taskId {
-				wg.Done()
-			}
-		})
-
-		err := downloader.Continue(&TaskFilter{IDs: []string{taskId}})
-		if err != nil {
-			// Task might have finished or not be in pausable state
-			t.Logf("Continue() note: %v", err)
-		}
-		wg.Wait()
-
-		task = downloader.GetTask(taskId)
-		if task != nil && task.Status != base.DownloadStatusDone {
-			t.Errorf("Task should be done, got %s", task.Status)
-		}
-	})
 
 	// Clean up
 	downloader.Delete(nil, true)
@@ -1603,61 +1544,6 @@ func TestDownloader_DeleteAll(t *testing.T) {
 	}
 }
 
-func TestDownloader_ContinueAll(t *testing.T) {
-	listener := test.StartTestFileServer()
-	defer listener.Close()
-
-	downloader := NewDownloader(nil)
-	if err := downloader.Setup(); err != nil {
-		t.Fatal(err)
-	}
-	defer downloader.Clear()
-
-	// Create a task and wait for completion
-	req := &base.Request{
-		URL: "http://" + listener.Addr().String() + "/" + test.BuildName,
-	}
-
-	var wg sync.WaitGroup
-	wg.Add(1)
-	downloader.Listener(func(event *Event) {
-		if event.Key == EventKeyDone {
-			wg.Done()
-		}
-	})
-
-	taskId, err := downloader.CreateDirect(req, &base.Options{
-		Path: test.Dir,
-		Name: test.DownloadName,
-		Extra: http.OptsExtra{
-			Connections: 4,
-		},
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	wg.Wait()
-
-	// Verify task completed
-	task := downloader.GetTask(taskId)
-	if task == nil {
-		t.Fatal("Task should exist")
-	}
-	if task.Status != base.DownloadStatusDone {
-		t.Errorf("Task should be done, got %s", task.Status)
-	}
-
-	// Test ContinueBatch with nil on an already done task (should be a no-op)
-	err = downloader.ContinueBatch(nil)
-	if err != nil {
-		t.Logf("ContinueBatch(nil) on done tasks returned: %v", err)
-	}
-
-	// Clean up
-	downloader.Delete(nil, true)
-}
-
 func TestDownloader_ProtocolConfigNotExist(t *testing.T) {
 	downloader := NewDownloader(nil)
 	if err := downloader.Setup(); err != nil {

pkg/download/extract.go

@@ -229,7 +229,7 @@ func extractArchive(archivePath string, destDir string, password string, progres
 func createCountingHandler(count *int) func(ctx context.Context, fileInfo archives.FileInfo) error {
 	return func(ctx context.Context, fileInfo archives.FileInfo) error {
 		if !fileInfo.IsDir() {
-			*count++
+			(*count)++
 		}
 		return nil
 	}

pkg/download/extract_test.go

@@ -227,11 +227,14 @@ func createTestZipWithMultipleFiles(path string, numFiles int) error {
 	w := zip.NewWriter(zipFile)
 
 	for i := 0; i < numFiles; i++ {
-		f, err := w.Create(filepath.Join("dir", "file"+string(rune('A'+i))+".txt"))
+		// Use Windows-safe names. The previous rune-based approach could generate
+		// invalid Windows filename characters (e.g. '\\', ':', '|') when numFiles is large.
+		nameInZip := fmt.Sprintf("dir/file%03d.txt", i+1)
+		f, err := w.Create(nameInZip)
 		if err != nil {
 			return err
 		}
-		_, err = f.Write([]byte("Content of file " + string(rune('A'+i))))
+		_, err = f.Write([]byte(fmt.Sprintf("Content of file %03d", i+1)))
 		if err != nil {
 			return err
 		}
@@ -1055,7 +1058,15 @@ func TestExtractArchive_ReadOnlyDestDir(t *testing.T) {
 		t.Fatal(err)
 	}
 	// Make it writable for cleanup
-	defer os.Chmod(destDir, 0755)
+	defer func() { _ = os.Chmod(destDir, 0755) }()
+
+	// Windows ACL semantics mean chmod often does not prevent writes.
+	// If the filesystem doesn't enforce the permission change, skip.
+	probe := filepath.Join(destDir, "__write_probe.tmp")
+	if err := os.WriteFile(probe, []byte("x"), 0644); err == nil {
+		_ = os.Remove(probe)
+		t.Skip("filesystem does not enforce read-only directory permissions")
+	}
 
 	// Extraction should fail due to read-only destination
 	err = extractArchive(zipPath, destDir, "", nil)
@@ -1589,9 +1600,14 @@ func TestGetMultiPartArchiveBaseName(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.filename, func(t *testing.T) {
-			result := GetMultiPartArchiveBaseName(tt.filename)
-			if result != tt.expected {
-				t.Errorf("GetMultiPartArchiveBaseName(%q) = %q, expected %q", tt.filename, result, tt.expected)
+			filename := filepath.FromSlash(tt.filename)
+			expected := tt.expected
+			if expected != "" {
+				expected = filepath.FromSlash(expected)
+			}
+			result := GetMultiPartArchiveBaseName(filename)
+			if result != expected {
+				t.Errorf("GetMultiPartArchiveBaseName(%q) = %q, expected %q", filename, result, expected)
 			}
 		})
 	}
@@ -2067,10 +2083,16 @@ func TestDetermineFirstPartPath(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			result := determineFirstPartPath(tt.dir, tt.baseName, tt.pattern)
-			if result != tt.expected {
+			dir := filepath.FromSlash(tt.dir)
+			expected := tt.expected
+			if expected != "" {
+				expected = filepath.FromSlash(expected)
+			}
+
+			result := determineFirstPartPath(dir, tt.baseName, tt.pattern)
+			if result != expected {
 				t.Errorf("determineFirstPartPath(%q, %q, %q) = %q, expected %q",
-					tt.dir, tt.baseName, tt.pattern, result, tt.expected)
+					dir, tt.baseName, tt.pattern, result, expected)
 			}
 		})
 	}

pkg/download/storage.go

@@ -2,11 +2,12 @@ package download
 
 import (
 	"encoding/json"
-	"go.etcd.io/bbolt"
 	"os"
 	"path/filepath"
 	"reflect"
 	"sync"
+
+	"go.etcd.io/bbolt"
 )
 
 type Storage interface {
@@ -45,22 +46,24 @@ func changeValue(p any, v any) {
 	}
 }
 
-var memData = make(map[string]map[string]any)
-
 type MemStorage struct {
 	lock *sync.RWMutex
+	data map[string]map[string]any
 }
 
 func NewMemStorage() *MemStorage {
 	return &MemStorage{
 		lock: &sync.RWMutex{},
+		data: make(map[string]map[string]any),
 	}
 }
 
 func (n *MemStorage) Setup(buckets []string) error {
+	n.lock.Lock()
+	defer n.lock.Unlock()
 	for _, bucket := range buckets {
-		if _, ok := memData[bucket]; !ok {
-			memData[bucket] = make(map[string]any)
+		if _, ok := n.data[bucket]; !ok {
+			n.data[bucket] = make(map[string]any)
 		}
 	}
 	return nil
@@ -69,7 +72,7 @@ func (n *MemStorage) Setup(buckets []string) error {
 func (n *MemStorage) Put(bucket string, key string, v any) error {
 	n.lock.Lock()
 	defer n.lock.Unlock()
-	if bucketData, ok := memData[bucket]; ok {
+	if bucketData, ok := n.data[bucket]; ok {
 		bucketData[key] = v
 	}
 	return nil
@@ -78,7 +81,7 @@ func (n *MemStorage) Put(bucket string, key string, v any) error {
 func (n *MemStorage) Get(bucket string, key string, v any) (bool, error) {
 	n.lock.RLock()
 	defer n.lock.RUnlock()
-	if dv, ok := memData[bucket][key]; ok {
+	if dv, ok := n.data[bucket][key]; ok {
 		changeValue(v, dv)
 		return true, nil
 	}
@@ -88,7 +91,7 @@ func (n *MemStorage) Get(bucket string, key string, v any) (bool, error) {
 func (n *MemStorage) List(bucket string, v any) error {
 	n.lock.RLock()
 	defer n.lock.RUnlock()
-	data := memData[bucket]
+	data := n.data[bucket]
 	list := make([]any, 0)
 	for _, v := range data {
 		list = append(list, v)
@@ -100,7 +103,7 @@ func (n *MemStorage) List(bucket string, v any) error {
 func (n *MemStorage) Pop(bucket string, key string, v any) error {
 	n.lock.Lock()
 	defer n.lock.Unlock()
-	data := memData[bucket]
+	data := n.data[bucket]
 	changeValue(v, data[key])
 	delete(data, key)
 	return nil
@@ -109,7 +112,7 @@ func (n *MemStorage) Pop(bucket string, key string, v any) error {
 func (n *MemStorage) Delete(bucket string, key string) error {
 	n.lock.Lock()
 	defer n.lock.Unlock()
-	delete(memData[bucket], key)
+	delete(n.data[bucket], key)
 	return nil
 }
 
@@ -120,7 +123,7 @@ func (n *MemStorage) Close() error {
 func (n *MemStorage) Clear() error {
 	n.lock.Lock()
 	defer n.lock.Unlock()
-	memData = make(map[string]map[string]any)
+	n.data = make(map[string]map[string]any)
 	return nil
 }
 

pkg/rest/server_test.go

@@ -216,6 +216,7 @@ func TestPauseAndContinueTask(t *testing.T) {
 		if t2.Status != base.DownloadStatusPause {
 			t.Errorf("PauseTask() got = %v, want %v", t2.Status, base.DownloadStatusPause)
 		}
+		time.Sleep(time.Millisecond * 100)
 		httpRequestCheckOk[any](http.MethodPut, "/api/v1/tasks/"+taskId+"/continue", nil)
 		t3 := httpRequestCheckOk[*download.Task](http.MethodGet, "/api/v1/tasks/"+taskId, nil)
 		if t3.Status != base.DownloadStatusRunning {
@@ -240,6 +241,7 @@ func TestPauseAllAndContinueALLTasks(t *testing.T) {
 
 		createAndPause := func() {
 			taskId := httpRequestCheckOk[string](http.MethodPost, "/api/v1/tasks", createReq)
+			time.Sleep(time.Millisecond * 5)
 			httpRequestCheckOk[*download.Task](http.MethodPut, "/api/v1/tasks/"+taskId+"/pause", nil)
 		}
 
@@ -247,6 +249,7 @@ func TestPauseAllAndContinueALLTasks(t *testing.T) {
 		for i := 0; i < total; i++ {
 			createAndPause()
 		}
+		time.Sleep(time.Millisecond * 50)
 
 		// continue all
 		httpRequestCheckOk[any](http.MethodPut, "/api/v1/tasks/continue", nil)