Handle socket error by catching badmatch

[kuenishi]

When a client cancelled or stopped uploading an object in the middle,
the socket for TCP connection returns error when mochiweb tried to
read payload. It had been throwing badmatch and that request was just
failing, leaving already uploaded chunks and writing manifests.

This commit adds a try-catch clause to catch the badmatch error thrown
by mochiweb and calls amendment code that moves the exact manifest to
GC bucket and marks it as scheduled_delete.

A test is included in regression_tests.erl, where a socket error is
emulated by client-side socket close in the middle of upload.

Partial solution for #770.

[shino]

CR+LF is more common for line break in HTTP?

[kuenishi]

As this is a test code and dumb client, I didn't care much about that.

[shino]

Nice test case 👍

[shino]

Interruptions for normal upload and copy upload work fine 👍
For copy upload for MP part, client socket close made error log, which does no harm:

Client side, execute the below command and stop it by Ctrl-C:

S3CURL=.s3curl.15018.alice s3curl.pl --id cs -- -X PUT -x 127.0.0.1:15018 -v -H 'Content-length: 0' -H 'x-amz-copy-source: /test/rand.100MB' -s  http://test.s3.amazonaws.com/mp'?uploadId=XelIvUl3T5uPP_Gnnj2b3A%3D%3D&partNumber=3'

Error log:

15:07:15.769 [error] gen_fsm <0.4528.0> in state not_full terminated with reason: no such process or port in call to gen_fsm:sync_send_event(undefined, {gc_specific_manifest,<<199,130,21,21,106,2,74,79,178,198,229,149,37,33,59,250>>}, infinity)
15:07:15.771 [error] CRASH REPORT Process <0.4528.0> with 1 neighbours exited with reason: no such process or port in call to gen_fsm:sync_send_event(undefined, {gc_specific_manifest,<<199,130,21,21,106,2,74,79,178,198,229,149,37,33,59,250>>}, infinity) in gen_fsm:terminate/7 line 622

[shino]

Maybe, the above error is caused when put fsm does not have manifest fsm in its state.
For multipart related requests, put fsm does not spawn manifest fsm as in:

maybe_riak_cs_manifest_fsm_start_link(false, _Bucket, _Key, _RcPid) ->
    {ok, undefined};
maybe_riak_cs_manifest_fsm_start_link(true, Bucket, Key, RcPid) ->
    riak_cs_manifest_fsm:start_link(Bucket, Key, RcPid).

[shino]

Now running riak test.

[shino]

Two cases failed in multibag flavor, local fix for them:

diff --git a/riak_test/tests/regression_tests.erl b/riak_test/tests/regression_tests.erl
index 9b1dbce..b8a320b 100644
--- a/riak_test/tests/regression_tests.erl
+++ b/riak_test/tests/regression_tests.erl
@@ -174,7 +174,7 @@ verify_cs770({UserConfig, {RiakNodes, _, _}}, BucketName) ->
                           scheduled_delete =:= Mx?MANIFEST.state
                   end, 8, 4096),

-    Pbc = rt:pbc('dev1@127.0.0.1'),
+    Pbc = rtcs:pbc(RiakNodes, objects, BucketName),

     %% verify that object is also stored in latest GC bucket
     Ms = all_manifests_in_gc_bucket(Pbc),
diff --git a/riak_test/tests/upgrade_downgrade_test.erl b/riak_test/tests/upgrade_downgrade_test.erl
index 5e82cfa..cff44d5 100644
--- a/riak_test/tests/upgrade_downgrade_test.erl
+++ b/riak_test/tests/upgrade_downgrade_test.erl
@@ -51,6 +51,9 @@ confirm() ->
          ok = rt:upgrade(RiakNode, RiakCurrentVsn),
          rt:wait_for_service(RiakNode, riak_kv),
          ok = rtcs_config:upgrade_cs(N, AdminCreds),
+         rtcs:set_advanced_conf({cs, current, N},
+                                [{riak_cs,
+                                  [{riak_host, {"127.0.0.1", rtcs_config:pb_port(1)}}]}]),
          rtcs_exec:start_cs(N, current)
      end
      || RiakNode <- RiakNodes],

[shino]

regression_tests is still failing... Not reproducible single test run with -t option of riak_test. But it fails with -d riak_test/ebin option... 😿

[shino]

During lunch, test run with -d option succeeded with the diff below:

diff --git a/riak_test/tests/regression_tests.erl b/riak_test/tests/regression_tests.erl
index 9b1dbce..aea3d94 100644
--- a/riak_test/tests/regression_tests.erl
+++ b/riak_test/tests/regression_tests.erl
@@ -156,7 +156,7 @@ verify_cs770({UserConfig, {RiakNodes, _, _}}, BucketName) ->
     {ok, Socket} = rtcs_object:upload(UserConfig,
                                       {normal_partial, 3*1024*1024, 1024*1024},
                                       BucketName, Key),
-
+timer:sleep(3*1000),
     [[{UUID, M}]] = get_manifests(RiakNodes, BucketName, Key),

But I'm not sure this is actually a race or not.

[kuenishi]

Great catch! That race condition of socket close and writing and getting manifest from Riak. I pushed fix.

[shino]

Diff looks nice, all riak_test passed. Really nice fix and a good example of "don't let it crash" 😄

[kuenishi]

@borshop merge

On Thursday, 14 January 2016, Shunichi Shinohara notifications@github.com
wrote:

Diff looks nice, all riak_test passed. Really nice fix and a good example
of "don't let it crash" [image: 😄]

—
Reply to this email directly or view it on GitHub
#1280 (comment).

Sent from Gmail Mobile