Remove setting net.Conn Deadlines as its not needed anymore #8269

harshavardhana · 2019-09-19T04:02:32Z

Description

Remove setting net.Conn Deadlines as it's not needed anymore

Motivation and Context

This commits fixes a bug introduced in af6c6a2.

Setting deadlines in Go results in arbitrary hangs as reported here
golang/go#34385

Fixes #7852

How to test this PR?

Just the code shared in the reproducer

<?php

// Include the SDK using the Composer autoloader
date_default_timezone_set('America/Los_Angeles');
require 'vendor/autoload.php';

$s3 = new Aws\S3\S3Client([
        'version' => 'latest',
        'region'  => 'us-east-1',
        'endpoint' => 'http://127.0.0.1:9001',
        'use_path_style_endpoint' => true,
        'credentials' => [
                'key'    => 'minio',
                'secret' => 'minio123',
        ],
]);

$path = "s3://testbucket/hosts";
        
$s3->registerStreamWrapper();

$data = file_get_contents($path);

// Print the body of the result by indexing into the result object.
print_r( $data );

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:

Fixes a regression introduced in af6c6a2
Documentation needed
Unit tests needed
Functional tests needed (If yes, add mint PR # here: )

vadmeste

LGTM

nitisht · 2019-09-19T13:13:30Z

waiting for mint-auto test run before merge

This commits fixes a bug introduced in af6c6a2. Setting deadlines in Go results in arbitrary hangs as reported here golang/go#34385 Fixes minio#7852

kannappanr · 2019-09-20T01:44:33Z

@balamurugana @vadmeste Can you please review again?

minio-ops · 2019-09-20T04:18:08Z

Mint Automation

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

8269-ff2be85/mint-compression-dist-xl.sh.log:

Running with
SERVER_ENDPOINT:      72.28.97.59:32089
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 745035fe6c7c:/mint/log /tmp/mint-logs'

(1/14) Running aws-sdk-go tests ... done in 0 seconds
(2/14) Running aws-sdk-java tests ... done in 1 seconds
(3/14) Running aws-sdk-php tests ... done in 44 seconds
(4/14) Running aws-sdk-ruby tests ... done in 3 seconds
(5/14) Running awscli tests ... done in 2 minutes and 11 seconds
(6/14) Running healthcheck tests ... done in 0 seconds
(7/14) Running mc tests ... done in 1 minutes and 0 seconds
(8/14) Running minio-dotnet tests ... done in 56 seconds
(9/14) Running minio-go tests ... done in 1 minutes and 15 seconds
(10/14) Running minio-java tests ... done in 1 minutes and 51 seconds
(11/14) Running minio-js tests ... FAILED in 30 minutes and 14 seconds
{
  "name": "minio-js",
  "function": "\"after all\" hook in \"functional tests\"",
  "duration": 30,
  "status": "FAIL",
  "error": "S3Error: The bucket you tried to delete is not empty at Object.parseError (node_modules/minio/dist/main/xml-parsers.js:83:11) at /mint/run/core/minio-js/node_modules/minio/dist/main/transformers.js:153:22 at DestroyableTransform._flush (node_modules/minio/dist/main/transformers.js:77:10) at DestroyableTransform.prefinish (node_modules/readable-stream/lib/_stream_transform.js:129:10) at prefinish (node_modules/readable-stream/lib/_stream_writable.js:607:14) at finishMaybe (node_modules/readable-stream/lib/_stream_writable.js:616:5) at endWritable (node_modules/readable-stream/lib/_stream_writable.js:629:3) at DestroyableTransform.Writable.end (node_modules/readable-stream/lib/_stream_writable.js:567:22) at IncomingMessage.onend (_stream_readable.js:514:10) at endReadableNT (_stream_readable.js:978:12) at _combinedTickCallback (internal/process/next_tick.js:80:11) at process._tickCallback (internal/process/next_tick.js:104:9)"
}

Executed 10 out of 14 tests successfully.

8269-ff2be85/mint-fs.sh.log:

Running with
SERVER_ENDPOINT:      72.28.97.58:32155
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 e312ca1e45c2:/mint/log /tmp/mint-logs'

(1/14) Running aws-sdk-go tests ... done in 0 seconds
(2/14) Running aws-sdk-java tests ... done in 1 seconds
(3/14) Running aws-sdk-php tests ... done in 42 seconds
(4/14) Running aws-sdk-ruby tests ... done in 2 seconds
(5/14) Running awscli tests ... done in 1 minutes and 54 seconds
(6/14) Running healthcheck tests ... done in 0 seconds
(7/14) Running mc tests ... done in 56 seconds
(8/14) Running minio-dotnet tests ... done in 29 seconds
(9/14) Running minio-go tests ... done in 47 seconds
(10/14) Running minio-java tests ... done in 54 seconds
(11/14) Running minio-js tests ... done in 47 seconds
(12/14) Running minio-py tests ... FAILED in 2 minutes and 32 seconds
{
  "error": "Traceback (most recent call last):\n  File \"/mint/run/core/minio-py/tests.py\", line 1341, in test_presigned_get_object_expiry_5sec\n    object_name).get_exception()\nminio.error.AccessDenied: AccessDenied: message: Access Denied\n\nDuring handling of the above exception, another exception occurred:\n\nTraceback (most recent call last):\n  File \"/mint/run/core/minio-py/tests.py\", line 2036, in main\n    test_presigned_get_object_expiry_5sec(client, log_output)\n  File \"/mint/run/core/minio-py/tests.py\", line 1355, in test_presigned_get_object_expiry_5sec\n    raise Exception(err)\nException: AccessDenied: message: Access Denied\n",
  "duration": 104,
  "status": "FAIL",
  "function": "presigned_get_object(bucket_name, object_name, expires, response_headers, request_date)",
  "name": "minio-py:test_presigned_get_object_expiry_5sec",
  "message": "AccessDenied: message: Access Denied",
  "args": {
    "response.reason": "Forbidden",
    "bucket_name": "minio-py-test-a05b5b60-37e0-492d-b84b-6b019501667c",
    "response.headers": "{\"_container\": {\"accept-ranges\": [\"Accept-Ranges\", \"bytes\"], \"content-length\": [\"Content-Length\", \"440\"], \"content-security-policy\": [\"Content-Security-Policy\", \"block-all-mixed-content\"], \"content-type\": [\"Content-Type\", \"application/xml\"], \"server\": [\"Server\", \"MinIO/DEVELOPMENT.2019-09-20T03-26-09Z\"], \"vary\": [\"Vary\", \"Origin\"], \"x-amz-request-id\": [\"X-Amz-Request-Id\", \"15C608E25C2CB304\"], \"x-xss-protection\": [\"X-Xss-Protection\", \"1; mode=block\"], \"date\": [\"Date\", \"Fri, 20 Sep 2019 03:48:08 GMT\"]}}",
    "response.status": 403,
    "object_name": "43547ea6-6ebb-41f6-ac1e-33db0be88a39",
    "presigned_get_object_url": "http://72.28.97.58:32155/minio-py-test-a05b5b60-37e0-492d-b84b-6b019501667c/43547ea6-6ebb-41f6-ac1e-33db0be88a39?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=minio%2F20190920%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20190920T034803Z&X-Amz-Expires=5&X-Amz-SignedHeaders=host&X-Amz-Signature=995fac48aebc4a061ff9c3ea3dadef11856c9759bdcb8af60f864d7df162ac21",
    "response._body": "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<Error><Code>AccessDenied</Code><Message>Request has expired</Message><Key>43547ea6-6ebb-41f6-ac1e-33db0be88a39</Key><BucketName>minio-py-test-a05b5b60-37e0-492d-b84b-6b019501667c</BucketName><Resource>/minio-py-test-a05b5b60-37e0-492d-b84b-6b019501667c/43547ea6-6ebb-41f6-ac1e-33db0be88a39</Resource><RequestId>15C608E25C2CB304</RequestId><HostId>c15e53f2-f4f8-4aeb-be8d-7183a2beee9a</HostId></Error>"
  }
}

Executed 11 out of 14 tests successfully.

vadmeste

Some comments

cmd/http/accountingconn.go

harshavardhana added the regression (fixed) label Sep 19, 2019

nitisht requested review from vadmeste and balamurugana September 19, 2019 04:48

balamurugana approved these changes Sep 19, 2019

View reviewed changes

vadmeste approved these changes Sep 19, 2019

View reviewed changes

harshavardhana added the do-not-close label Sep 19, 2019

harshavardhana force-pushed the conn branch 2 times, most recently from a94e0a3 to ff2be85 Compare September 20, 2019 01:13

harshavardhana changed the title ~~Add a work-around to fix the SetReadDeadline() bug~~ Remove setting net.Conn Deadlines as its not needed anymore Sep 20, 2019

Remove setting net.Conn Deadlines as its not needed anymore

396ded3

This commits fixes a bug introduced in af6c6a2. Setting deadlines in Go results in arbitrary hangs as reported here golang/go#34385 Fixes minio#7852

harshavardhana force-pushed the conn branch from ff2be85 to 396ded3 Compare September 20, 2019 01:14

harshavardhana removed the do-not-close label Sep 20, 2019

kannappanr requested review from balamurugana and vadmeste September 20, 2019 01:44

balamurugana approved these changes Sep 20, 2019

View reviewed changes

vadmeste reviewed Sep 20, 2019

View reviewed changes

cmd/http/accountingconn.go Show resolved Hide resolved

cmd/http/accountingconn.go Show resolved Hide resolved

kannappanr merged commit 4780fa5 into minio:master Sep 20, 2019

harshavardhana deleted the conn branch September 20, 2019 18:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove setting net.Conn Deadlines as its not needed anymore #8269

Remove setting net.Conn Deadlines as its not needed anymore #8269

harshavardhana commented Sep 19, 2019 •

edited

Loading

vadmeste left a comment

nitisht commented Sep 19, 2019

kannappanr commented Sep 20, 2019

minio-ops commented Sep 20, 2019

vadmeste left a comment

Remove setting net.Conn Deadlines as its not needed anymore #8269

Remove setting net.Conn Deadlines as its not needed anymore #8269

Conversation

harshavardhana commented Sep 19, 2019 • edited Loading

Description

Motivation and Context

How to test this PR?

Types of changes

Checklist:

vadmeste left a comment

Choose a reason for hiding this comment

nitisht commented Sep 19, 2019

kannappanr commented Sep 20, 2019

minio-ops commented Sep 20, 2019

Mint Automation

8269-ff2be85/mint-compression-dist-xl.sh.log:

8269-ff2be85/mint-fs.sh.log:

vadmeste left a comment

Choose a reason for hiding this comment

harshavardhana commented Sep 19, 2019 •

edited

Loading