allow promises to fail #2257
allow promises to fail #2257
Conversation
|
@noffle could I get you to take a look at this one? |
| @@ -184,6 +184,9 @@ func (ds *dagService) GetNodes(ctx context.Context, keys []key.Key) []NodeGetter | |||
| select { | |||
| case blk, ok := <-blkchan: | |||
| if !ok { | |||
| for _, p := range promises { | |||
| p.Fail(ErrNotFound) | |||
| } | |||
There was a problem hiding this comment.
This fails all NodeGetters, right? Why can't we just let the failing ones fail and return the ones that do arrive?
There was a problem hiding this comment.
anything that does arrive will arrive by the time we get here
There was a problem hiding this comment.
Oh, I see! !ok is true when the channel has been closed. Yes, that makes good sense. :)
I think I see a nasty edge case here though: consider the scenario where some NodeGetters have their Node successfully sent to their recv channel, but nobody has called Get() on them yet. The above code executes, sending down ErrNotFound to the err channel of all NodeGetters -- even the ones that previously succeeded. So now you have some NodeGetters with data in both the recv channel and the err channel. However, select chooses at random when there are multiple cases that can run. This means there's a chance that calling Get() on a good NodeGetter will yield an error.
This is a long winded say of saying "we should probably only fail the NodeGetters that actually failed rather than all of them".
whyrusleeping
force-pushed
the
feat/promise-fail
branch
from
ff3373a
to
46152f2
Compare
whyrusleeping
force-pushed
the
feat/promise-fail
branch
from
46152f2
to
4d0a37a
Compare
whyrusleeping
force-pushed
the
feat/promise-fail
branch
from
817af36
to
11eba04
Compare
License: MIT Signed-off-by: Jeromy <jeromyj@gmail.com>
License: MIT Signed-off-by: Jeromy <jeromyj@gmail.com>
whyrusleeping
force-pushed
the
feat/promise-fail
branch
from
11eba04
to
b5a78b1
Compare
License: MIT Signed-off-by: Jeromy <why@ipfs.io>
whyrusleeping
force-pushed
the
feat/promise-fail
branch
from
b5a78b1
to
5a0b6e5
Compare
This allows the nodepromise thing to fail. In cases such as
ipfs refs -rrun offline when not all blocks are local, ipfs currently hangs.License: MIT
Signed-off-by: Jeromy jeromyj@gmail.com