Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set S3 Select record message length to 128KiB #7475

Merged
merged 1 commit into from Apr 4, 2019

Conversation

donatello
Copy link
Member

  • Previously this limit was a little more than 1MiB, and it broke
    compatibility with AWS SDK Java causing a buffer overflow error.

Regression

No

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added unit tests to cover my changes.
  • I have added/updated functional tests in mint. (If yes, add mint PR # here: )
  • All new and existing tests passed.

- Previously this limit was a little more than 1MiB, and it broke
  compatibility with AWS SDK Java causing a buffer overflow error.
@minio-ops
Copy link

Mint Automation

Test Result
mint-xl.sh ✔️
mint-compression-dist-xl.sh ✔️
mint-compression-fs.sh ✔️
mint-worm.sh ✔️
mint-fs.sh ✔️
mint-gateway-nas.sh ✔️
mint-compression-xl.sh more...
mint-large-bucket.sh more...
mint-dist-xl.sh more...

7475-687268b/mint-dist-xl.sh.log:

Running with
SERVER_ENDPOINT:      72.28.97.52:31806
ACCESS_KEY:           minio
SECRET_KEY:           ***REDACTED***
ENABLE_HTTPS:         0
SERVER_REGION:        us-east-1
MINT_DATA_DIR:        /mint/data
MINT_MODE:            full
ENABLE_VIRTUAL_STYLE: 0

To get logs, run 'docker cp 086880f8b25a:/mint/log /tmp/mint-logs'

(1/14) Running aws-sdk-go tests ... FAILED in 0 seconds
{
  "alert": "",
  "args": {
    "bucketName": "aws-sdk-go-test-dpvtg1qtkurgoh",
    "expiry": 60000000000,
    "objectName": "presignedTest"
  },
  "duration": 326,
  "error": "RequestError: send request failed\ncaused by: Put http://72.28.97.52:31806/aws-sdk-go-test-dpvtg1qtkurgoh: dial tcp 72.28.97.52:31806: connect: connection refused",
  "function": "PresignedPut",
  "message": "AWS SDK Go CreateBucket Failed",
  "name": "aws-sdk-go",
  "status": "FAIL"
}

Executed 0 out of 14 tests successfully.

7475-687268b/mint-compression-xl.sh.log:

Running with
SERVER_ENDPOINT:      72.28.97.61:30747
ACCESS_KEY:           minio
SECRET_KEY:           ***REDACTED***
ENABLE_HTTPS:         0
SERVER_REGION:        us-east-1
MINT_DATA_DIR:        /mint/data
MINT_MODE:            full
ENABLE_VIRTUAL_STYLE: 0

To get logs, run 'docker cp 7722ab5223c9:/mint/log /tmp/mint-logs'

(1/14) Running aws-sdk-go tests ... FAILED in 0 seconds
{
  "alert": "",
  "args": {
    "bucketName": "aws-sdk-go-test-rjvwxq3acdahob",
    "expiry": 60000000000,
    "objectName": "presignedTest"
  },
  "duration": 259,
  "error": "RequestError: send request failed\ncaused by: Put http://72.28.97.61:30747/aws-sdk-go-test-rjvwxq3acdahob: dial tcp 72.28.97.61:30747: connect: connection refused",
  "function": "PresignedPut",
  "message": "AWS SDK Go CreateBucket Failed",
  "name": "aws-sdk-go",
  "status": "FAIL"
}

Executed 0 out of 14 tests successfully.

7475-687268b/mint-large-bucket.sh.log:

Running with
SERVER_ENDPOINT:      72.28.97.60:30390
ACCESS_KEY:           minio
SECRET_KEY:           ***REDACTED***
ENABLE_HTTPS:         0
SERVER_REGION:        us-east-1
MINT_DATA_DIR:        /mint/data
MINT_MODE:            full
ENABLE_VIRTUAL_STYLE: 0

To get logs, run 'docker cp 4f6741ac3ac9:/mint/log /tmp/mint-logs'

(1/14) Running aws-sdk-go tests ... FAILED in 0 seconds
{
  "alert": "",
  "args": {
    "bucketName": "aws-sdk-go-test-p15fhg3ajpnwsz",
    "expiry": 60000000000,
    "objectName": "presignedTest"
  },
  "duration": 294,
  "error": "RequestError: send request failed\ncaused by: Put http://72.28.97.60:30390/aws-sdk-go-test-p15fhg3ajpnwsz: dial tcp 72.28.97.60:30390: connect: connection refused",
  "function": "PresignedPut",
  "message": "AWS SDK Go CreateBucket Failed",
  "name": "aws-sdk-go",
  "status": "FAIL"
}

Executed 0 out of 14 tests successfully.

@codecov
Copy link

codecov bot commented Apr 3, 2019

Codecov Report

Merging #7475 into master will increase coverage by <.01%.
The diff coverage is 73.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7475      +/-   ##
==========================================
+ Coverage   48.05%   48.06%   +<.01%     
==========================================
  Files         296      296              
  Lines       46725    46732       +7     
==========================================
+ Hits        22453    22460       +7     
  Misses      22198    22198              
  Partials     2074     2074
Impacted Files Coverage Δ
pkg/s3select/message.go 59.23% <73.33%> (+1.61%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 313a3a2...687268b. Read the comment docs.

Copy link
Contributor

@sinhaashish sinhaashish left a comment

Choose a reason for hiding this comment

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

LGTM

@kannappanr kannappanr merged commit b1b1d77 into minio:master Apr 4, 2019
klauspost added a commit to klauspost/minio that referenced this pull request Sep 25, 2019
Updates minio#7475

The Java implementation has a 128KB buffer and a message must be emitted before that is used. minio#7475 therefore limits the message size to 128KB. But up to 256 bytes are written to the buffer in each call. This means we must emit a message before shorter than 128KB.

Therefore we change the limit to 128KB minus 256 bytes.
kannappanr pushed a commit that referenced this pull request Sep 25, 2019
Updates #7475

The Java implementation has a 128KB buffer and a message must be emitted before that is used. #7475 therefore limits the message size to 128KB. But up to 256 bytes are written to the buffer in each call. This means we must emit a message before shorter than 128KB.

Therefore we change the limit to 128KB minus 256 bytes.
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.

None yet

5 participants