Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove client timeout handling and use SDK timeout support instead #10

Closed
davidkel opened this issue Oct 19, 2021 · 6 comments
Closed
Assignees

Comments

@davidkel
Copy link
Owner

davidkel commented Oct 19, 2021

We should move our test client to use the official timeouts. What would be good to test as well would be that the client doesn't remain hanging on exit (it would mean changing the client to just exit the loop when ctrl-c is received but not doing a process.exit() ) if timeouts or problems are hit

@bestbeforetoday
Copy link

After tonight's publish (0.1.0-dev.20211028.*) there should be full support for setting both default and (if necessary) per-call timeouts with the Node client. See samples for how to set defaults. Any feedback based on actual usage is appreciated.

@davidkel
Copy link
Owner Author

davidkel commented Nov 2, 2021

Taken from the sample, the code shows

   const gateway = connect({
        client,
        identity: await newIdentity(),
        signer: await newSigner(),
        // Default timeouts for different gRPC calls
        evaluateOptions: () => {
            return { deadline: Date.now() + 5000 }; // 5 seconds
        },
        endorseOptions: () => {
            return { deadline: Date.now() + 15000 }; // 15 seconds
        },
        submitOptions: () => {
            return { deadline: Date.now() + 5000 }; // 5 seconds
        },
        commitStatusOptions: () => {
            return { deadline: Date.now() + 60000 }; // 1 minute
        },
        chaincodeEventsOptions: () => {
            return { deadline: Date.now() + 60000 }; // 1 minute
        },
    });

@davidkel davidkel removed the blocked label Nov 2, 2021
@bestbeforetoday
Copy link

I would suggest not setting a default gRPC timeout in chaincodeEventsOptions. This is a streaming call so I don't think an arbitrary timeout for the call to complete is particularly advisable. The returned iterator has a close() method on it that you can use to cancel the call if you don't want to wait more than some time period for an event to arrive, or you can set a deadline for a specific chaincode events request explicitly in the call options for that call if you know your particular usage should have processed any required events and closed the iterator within a certain time period.

@davidkel
Copy link
Owner Author

davidkel commented Nov 3, 2021

Currently the client has no timeout control on endorse, evaluate or submit so we can take advantage of the timeout support in the api now. We can remove our timeout implementation around getting status now and take advantage of the sdk timeout, however we should leave our timeout processing for chaincode events in place. The chaincode event options isn't applicable to our implementation.

@sapthasurendran
Copy link
Collaborator

sapthasurendran commented Nov 4, 2021

message":"9 FAILED_PRECONDITION: context deadline exceeded Details: []"
message":"4 DEADLINE_EXCEEDED: Deadline exceeded Details: []"

@bestbeforetoday Could you please help me differentiate these error messages during commit timeout ?

@davidkel
Copy link
Owner Author

davidkel commented Nov 4, 2021

#54

@davidkel davidkel closed this as completed Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants