Project

General

Profile

Actions

Bug #12247

closed

[keepproxy] Fix misleading log message when GET response is interrupted

Added by Tom Clegg over 7 years ago. Updated about 7 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Keep
Target version:
Start date:
09/13/2017
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-

Description

Reported on mailing list:

Sep 13 11:21:12 arvados keepproxy: 2017/09/13 11:21:12 128.248.171.19,172.17.0.231:52236 GET /fe194b0a4576d5c8d9d0a2055b132b60+67108864+Ab7cbd46f674287a7e906d009c18227475a83edbb@59cbcfe9 200 67108864 34647446 http://arvados-keep01.cri.local:25107/fe194b0a4576d5c8d9d0a2055b132b60+67108864+Ab7cbd46f674287a7e906d009c18227475a83edbb@59cbcfe9 Reader failed checksum
Sep 13 11:21:29 arvados keepproxy: 2017/09/13 11:21:29 128.248.171.19,172.17.0.231:51688 GET /fe194b0a4576d5c8d9d0a2055b132b60+67108864+Ab7cbd46f674287a7e906d009c18227475a83edbb@59cbcfe9 200 67108864 30612102 http://arvados-keep01.cri.local:25107/fe194b0a4576d5c8d9d0a2055b132b60+67108864+Ab7cbd46f674287a7e906d009c18227475a83edbb@59cbcfe9 Reader failed checksum
Sep 13 11:21:44 arvados keepproxy: 2017/09/13 11:21:44 128.248.171.19,172.17.0.231:50472 GET /fe194b0a4576d5c8d9d0a2055b132b60+67108864+Ab7cbd46f674287a7e906d009c18227475a83edbb@59cbcfe9 200 67108864 34956478 http://arvados-keep01.cri.local:25107/fe194b0a4576d5c8d9d0a2055b132b60+67108864+Ab7cbd46f674287a7e906d009c18227475a83edbb@59cbcfe9 Reader failed checksum
Sep 13 11:22:00 arvados keepproxy: 2017/09/13 11:22:00 128.248.171.19,172.17.0.231:52286 GET /fe194b0a4576d5c8d9d0a2055b132b60+67108864+Ab7cbd46f674287a7e906d009c18227475a83edbb@59cbcfe9 200 67108864 63292254 http://arvados-keep01.cri.local:25107/fe194b0a4576d5c8d9d0a2055b132b60+67108864+Ab7cbd46f674287a7e906d009c18227475a83edbb@59cbcfe9 Reader failed checksum

The corresponding keepstore logs indicated everything was fine.

Suspect the following code in source:sdk/go/keepclient/hashcheck.go:

func (this HashCheckingReader) WriteTo(dest io.Writer) (written int64, err error) {
    if writeto, ok := this.Reader.(io.WriterTo); ok {
        written, err = writeto.WriteTo(io.MultiWriter(dest, this.Hash))
    } else {
        written, err = io.Copy(io.MultiWriter(dest, this.Hash), this.Reader)
    }

    sum := this.Hash.Sum(make([]byte, 0, this.Hash.Size()))

    if fmt.Sprintf("%x", sum) != this.Check {
        err = BadChecksum
    }

If io.Copy returned a non-nil error, then we should return that error and skip the hash check, like we do in Read().

I expect the resulting log message will be something like "write to closed socket", which would be a much better clue that the problem (if any) is just about sending the response to this particular request, and there's no reason to suspect data corruption.


Subtasks 1 (0 open1 closed)

Task #12263: Review 12247-no-failed-checksumResolvedTom Clegg09/13/2017

Actions
Actions #1

Updated by Tom Clegg over 7 years ago

Reproduced with curl on a dev cluster:

curl -H 'Authorization: OAuth2 [...]' 'http://localhost:9100/700c896930bd070fe7c8fa7344393bbd+1624300+A0a49f7074eb95caf1aeede24dfab1dbc79f598dc@59cbd944' | head -c1

2017-09-13_17:02:36.08291 2017/09/13 17:02:36 [::1]:59214 GET /700c896930bd070fe7c8fa7344393bbd+1624300+A0a49f7074eb95caf1aeede24dfab1dbc79f598dc@59cbd944 200 1624300 36743 http://keep0.4xphq.arvadosapi.com:25107/700c896930bd070fe7c8fa7344393bbd+1624300+A0a49f7074eb95caf1aeede24dfab1dbc79f598dc@59cbd944 Reader failed checksum
Actions #2

Updated by Tom Clegg over 7 years ago

  • Description updated (diff)
Actions #3

Updated by Tom Morris over 7 years ago

  • Target version set to 2017-09-27 Sprint
Actions #4

Updated by Tom Clegg over 7 years ago

  • Assigned To set to Tom Clegg
Actions #5

Updated by Tom Clegg over 7 years ago

  • Status changed from New to In Progress
Actions #6

Updated by Tom Clegg over 7 years ago

12247-no-failed-checksum @ 8d9adcff3cfc57c775f136ef32dcad6b3860a5a3

Actions #7

Updated by Lucas Di Pentima over 7 years ago

Ran sdk/go/keepclient tests successfully. This LGTM, thanks.

Actions #8

Updated by Anonymous over 7 years ago

  • Status changed from In Progress to Resolved
  • % Done changed from 0 to 100

Applied in changeset arvados|commit:fb79ddbedcc8c459daccaa6eec05b07206865bae.

Actions

Also available in: Atom PDF