fs, memory, s3 marker: take context cancellation into account

[ahouene]

All function for backends fs and memory do not use their ctx parameter. This adds checks for ctx.Err() at the beginning of those.

As for s3, check ctx from marker logic even if marker is disabled.

[johanlantz]

I am struggling a little to understand why ctx.Err() is returned here. The doc above says:
// In case the UseUpdateMarker option is false, this function doesn't do
// anything and returns no error

so returning nil seems to better match that description?

[johanlantz]

Added two comments but none that prevents a merge from my side.

[wojas]

Contexts exist to cancel or prevent long expensive operations, typically network operations.

The context package introduction puts it like this:

Package context defines the Context type, which carries deadlines, cancellation signals, and other request-scoped values across API boundaries and between processes. Incoming requests to a server should create a Context, and outgoing calls to servers should accept a Context.

None of the os.* functions for filesystem access take any context, and checking it in the fs backend feels a bit artificial. For the memory backend it is definitely quite unconventional.

Is there a practical reason for adding all these boilerplate checks?

[ahouene]

The idea behind that is to honour the ctx parameter, since it is present in the function signatures regardless of having an actual networked/concurrent backend implementation.

Consider the following code:

var storage simpleblob.Interface
// ...
ctx, cancel := context.WithCancel(context.Background())
cancel()
err := storage.Delete(ctx, "foo")

To the question Has the blob named "foo" been deleted and do we have an error?, the answer is currently It depends on the backend, and this PR aims to make the answer be independent of the backend: An error was returned and the blob was not deleted.

More generally, if I call err := f(ctx) I expect err to be non-nil if ctx is cancelled. Incidentally, the current implementation makes it up to the person using simpleblob to write the boilerplate checks to make sure that the context is indeed not cancelled.

[wojas]

Typically cancellation occurs asynchronously from the operation that is covered by the context. If you check at the start of a function and expect that to prevent execution, you are basically introducing a race condition.

One cannot expect that just because a function takes a context as a parameter, the function will not do anything if the context is already cancelled. Context were originally an optimization to prevent unnecessary when a client connection was already closed for whatever reason.

If the caller wants to ensure that the following code is not executed if the context is already closed, it needs to explicitly perform that check. You cannot expect every single function that receives a context to check it before continuing execution. The context is passed through the stack so that any deeper layer can use it for cancellation, but there is no requirement to do so. If it were, Go code would have to have such boilerplate checks all over the place.