Skip to content

add retry for http retryable requests#1390

Merged
harshavardhana merged 1 commit intominio:masterfrom
prakashsvmx:retry-req
Mar 13, 2025
Merged

add retry for http retryable requests#1390
harshavardhana merged 1 commit intominio:masterfrom
prakashsvmx:retry-req

Conversation

@prakashsvmx
Copy link
Copy Markdown
Member

@prakashsvmx prakashsvmx commented Mar 12, 2025 •
edited
Loading

add retry for http retryable requests

e.g:

import * as fs from 'node:fs'
import * as Minio from 'minio'
import * as stream from 'stream'

var minioClient = new Minio.Client({
  endPoint: 'localhost',
  port: 22000,
  useSSL: false,
  accessKey: 'minio',
  secretKey: 'minio123',
  region: 'us-west-1',
  //pathStyle:true
})

const retryTest = async () => {
  try {
    const filePath = '/home/prakash/Downloads/test-all-file-types/Sizes/1GB.zip'
    const stream = fs.createReadStream(filePath, { highWaterMark: 1024 * 1024 })
    const { size } = await fs.promises.stat(filePath)
    const result = await minioClient.putObject('test-bucket', '1GB.zip', stream, size)
    console.log('Upload success ', result)
  } catch (err) {
    console.log('Error in caller:', err.message)
  }
}

retryTest()
# Try to make few drives offline or permission denied. to simulate it

chmod 000 ./data{1..3}/
or revert back the permission like:
chmod 755 -R ./data{1..3}/

Sample runs.


node examples/put-object.mjs

HTTP Retryable: 503 Attempt: 0
Retrying request (attempt 1/3) after 239.0578579013787ms due to: Error: Retryable HTTP status: 503
HTTP Retryable: 503 Attempt: 1
Retrying request (attempt 2/3) after 440.8090880709857ms due to: Error: Retryable HTTP status: 503
HTTP Retryable: 503 Attempt: 2
Retrying request (attempt 3/3) after 1078.2877396516749ms due to: Error: Retryable HTTP status: 503
HTTP Retryable: 503 Attempt: 3
Error: Request failed after 3 retries: Error: Retryable HTTP status: 503


node examples/put-object.mjs

HTTP Retryable: 503 Attempt: 0
Retrying request (attempt 1/3) after 238.83703937313183ms due to: Error: Retryable HTTP status: 503
HTTP Retryable: 503 Attempt: 1
Retrying request (attempt 2/3) after 474.3181184853617ms due to: Error: Retryable HTTP status: 503
Upload success  {
  etag: '286e80b3b7420263038ab06d76774043',
    versionId: '76ebf6ad-4242-44bb-9083-6cbbec528621'
}

@prakashsvmx prakashsvmx self-assigned this Mar 12, 2025
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 12, 2025
View reviewed changes
@prakashsvmx prakashsvmx marked this pull request as ready for review March 13, 2025 02:31
@harshavardhana
Copy link
Copy Markdown
Member

harshavardhana commented Mar 13, 2025

🍰

@harshavardhana harshavardhana merged commit 0e6f582 into minio:master Mar 13, 2025
14 checks passed
This was referenced Jun 10, 2025
Closed
Open
Closed
@aldy505
Copy link
Copy Markdown
Contributor

aldy505 commented Sep 19, 2025

Honestly this is a very bad change. There are no way to configure maximum retry attempts and no way to configure timeout. I had production issues because of this.

@prakashsvmx
Copy link
Copy Markdown
Member Author

prakashsvmx commented Sep 19, 2025

now that we have the functionality and most of them are constants which can be extracted into client arguments for customisation. it needs some work yes. feel free to send a PR.

aldy505
aldy505 reviewed Sep 19, 2025
View reviewed changes
src/internal/request.ts
transport: Transport,
opt: https.RequestOptions,
body: Buffer | string | stream.Readable | null = null,
maxRetries: number = MAX_RETRIES,
Copy link
Copy Markdown
Contributor

@aldy505 aldy505 Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This option is never respected and never called.

src/internal/request.ts
}

const MAX_RETRIES = 10
const EXP_BACK_OFF_BASE_DELAY = 1000 // Base delay for exponential backoff
Copy link
Copy Markdown
Contributor

@aldy505 aldy505 Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Base delay of 1 second is not a good value. Based on getExpBackoffDelay function, this will have a maximum value around 1009556 milliseconds, which is 16 minutes.

@aldy505
Copy link
Copy Markdown
Contributor

aldy505 commented Sep 19, 2025

now that we have the functionality and most of them are constants which can be extracted into client arguments for customisation. it needs some work yes. feel free to send a PR.

@prakashsvmx I will send a PR in a few hours. But what about the release? Can you put out a release shortly after that?

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

Reviewers

@harshavardhana harshavardhana harshavardhana approved these changes

+1 more reviewer

@aldy505 aldy505 aldy505 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@prakashsvmx prakashsvmx

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@prakashsvmx @harshavardhana @aldy505 @github-advanced-security