client: Stats of failed object GET/RANGE/SEARCH are submitted only on Close

[cthulhu-rider]

currently, server-streamed ops call stat handler only when user calls Close. But for such ops Close is only used when user is no longer interested in the stream. Unlike PUT when user signals that data is finished via Close. For example, following code looks completely OK:

r, err := c.ObjectSearchInit(ctx, anyCID, anyValidSigner, anyValidOpts)
for err == nil {
	_, err = r.Read([]oid.ID{{}})
}
return err // io.EOF or failure

Current Behavior

-stats

Expected Behavior

+stats

Possible Solution

do not require calling Close. Submit stats whenever stream is finished (io.EOF==OK!). Update docs

Steps to Reproduce

• see exec statistics cases from Test coverage for Client ops #641

Regression

quite possible, but i havent checked

Your Environment

• Version of the product used: 335d9fe

[roman-khimov]

That was a deliberate choice because previously this statistics was "collected" for each r.Read() call. Tying it to Close() was the easiest way to go. The main question here --- is Close() optional? It's not to me, we're providing io.ReaderCloser, most of the time people are supposed to call Close() to release some resource in this case.

[cthulhu-rider]

@roman-khimov i meant this in terms of stats. We can submit real stats when stream is done and Read() returns an error regardless of closing. This is what i actually propose. I just noticed Failure reason can be received via Close. in docs, and this is not quire true. After Read() returns an error, Close() is no-op and returns the same

overall, i agree that closer is nice thing todo. For reading ops, it should be performed deferly. According to gRPC server-side stream semantics, currently it is always no-op except when user interrupts reading when data is no longer needed

[roman-khimov]

We can do a lot of things, the question is should we? To me it works fine already wrt statistics. Users must Close() and that's where you get your overall statistics.

[cthulhu-rider]

Users must Close()

the imposition is excessive, the stream works differently. UX improvement is desirable imo. Being a user, i'd be grateful for the stats that came even if I forgot closer

[roman-khimov]

I doubt anyone cares much about streams that drive all of this. But people know https://pkg.go.dev/os#File, they know https://pkg.go.dev/io#ReadCloser, not calling Close() for os.File is an error, for io.ReadCloser the general expectation is the same.

[roman-khimov]

Body in https://pkg.go.dev/net/http#Response is the same, btw. It is an error not to Close() it.

[cthulhu-rider]

I doubt anyone cares much about streams that drive all of this. But people know https://pkg.go.dev/os#File, they know https://pkg.go.dev/io#ReadCloser, not calling Close() for os.File is an error, for io.ReadCloser the general expectation is the same.

interface itself states no such requirements. 'Specific implementations may document their own behavior' and that's it

op stats and resource release in different planes to me. Exactly like successful reading 1GB data from file and forgotten descriptor