Skip to content

Fix retry mechanism - #1243

Open
agrasth wants to merge 2 commits into
masterfrom
retryFix
Open

Fix retry mechanism#1243
agrasth wants to merge 2 commits into
masterfrom
retryFix

Conversation

@agrasth

@agrasth agrasth commented Oct 9, 2025

Copy link
Copy Markdown
Contributor
  • All tests passed. If this feature is not already covered by the tests, I added new tests.
  • All static analysis checks passed.
  • This pull request is on the master branch.
  • I used gofmt for formatting the code before submitting the pull request.

1. Fixed Critical Retry Bug (jfrog-client-go/utils/io/content/contentreader.go)

Why: Enable actual retries by returning true for retryable Windows errors

Before:

return false, errorutils.CheckError(err)  // Never retries!

After:

if runtime.GOOS == "windows" {
    if pathErr, ok := err.(*os.PathError); ok {
        if errno, ok := pathErr.Err.(syscall.Errno); ok {
            // ERROR_SHARING_VIOLATION=32, ERROR_LOCK_VIOLATION=33, ERROR_ACCESS_DENIED=5
            if errno == 32 || errno == 33 || errno == 5 {
                return true, err  // ✅ Actually retry!
            }
        }
    }
}

Why check these error codes:

  • 32 (ERROR_SHARING_VIOLATION): File is open by another process
  • 33 (ERROR_LOCK_VIOLATION): File is locked
  • 5 (ERROR_ACCESS_DENIED): Can happen when antivirus is scanning

2. Added Exponential Backoff (jfrog-client-go/utils/retryexecutor.go)

Why: Customer has aggressive antivirus that may hold locks for longer periods

What:

  • Start: 100ms delay
  • Progression: 100ms → 200ms → 400ms → 800ms → 1s → 1s → 1s...
  • Max: 1000ms (as requested by customer requirements)
  • Total: Up to ~7.5 seconds across 10 retries

Code:

type RetryExecutor struct {
    MaxRetries               int
    RetriesIntervalMilliSecs int
    UseExponentialBackoff    bool  // NEW
    MaxBackoffMilliSecs      int   // NEW
    // ...
}

3. Increased Retry Count (both files)

Why: Handle aggressive antivirus that scans files longer

Change: 5 retries → 10 retries
Impact:

  • Before: ~1 second max total retry time (5 × 200ms)
  • After: ~7.5 seconds max total retry time (100+200+400+800+1000×6)

4. Applied Same Fix to RemoveTempDir (jfrog-client-go/utils/io/fileutils/temp.go)

Why: Same Windows file locking issue affects directory removal

Implementation: Created removeWithRetry() helper function with same logic

@agrasth
agrasth requested a review from bhanurp October 9, 2025 07:58
@ehl-jf ehl-jf added the safe to test Approve running integration tests on a pull request label Oct 9, 2025
@github-actions github-actions Bot removed the safe to test Approve running integration tests on a pull request label Oct 9, 2025
@ehl-jf ehl-jf added the safe to test Approve running integration tests on a pull request label Oct 13, 2025
@github-actions github-actions Bot removed the safe to test Approve running integration tests on a pull request label Oct 13, 2025
@github-actions

Copy link
Copy Markdown
Contributor

👍 Frogbot scanned this pull request and did not find any new security issues.


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants