[sdk#1026] Use postpone.ContextWithValues()

[Bolodya1997]

Description

Use postpone.ContextWithValues() for Close/Unregister in failed Request/Register cases.

Depends on networkservicemesh/sdk#1035.

Issue link

networkservicemesh/sdk#1026

How Has This Been Tested?

• Added unit testing to cover
• Tested manually
• Tested by integration testing
• Have not tested

Types of changes

• Bug fix
• New functionallity
• Documentation
• Refactoring
• CI

[pperiyasamy]

@Bolodya1997 I'm not sure if I fully understand this solution, are you trying to postpone the invocation of i.Close with closeCtx ? what is the purpose of this ?

[Bolodya1997]

We have met an issue in our integration tests - networkservicemesh/sdk#1026.
In short - sometimes Request context can be expired/canceled and so Close on failure cannot reach next hop.
So here it is actually a pattern we are planning to use to solve this issue.

// 1. Capture ctx values and timeout before the Request.
postponeCtxFunc := postpone.ContextWithValues(ctx)

// 2. Perform Request.
conn, err = next.Client(ctx).Request(ctx, request)
if err != nil {
    return nil, err
}

if err := doSomething(); err != nil {
    // 3. Create Close context using stored timeout, values.
    closeCtx, cancelClose := postponeCtxFunc()
    defer cancelClose()

    // 4. Perform Close using Close context.
    if _, closeErr := i.Close(closeCtx, conn, opts...); closeErr != nil {
        err = errors.Wrapf(err, "connection closed with error: %s", closeErr.Error())
    }
    return nil, err
}

return conn, nil

[pperiyasamy]

oh! ok, interesting change. is i.Close invoked with closeCtx which has extended deadline ?
should cancelClose() be called at the end ?

[Bolodya1997]

Yes closeCtx has postponed deadline in comparison with ctx.
cancelClose() is called with defer, so closeCtx would be gracefully closed.

[pperiyasamy]

ah! yes, correct. with this closeCtx's Done channel closed gracefully only after executing new hop's close handlers.