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

CLI times out too quickly #3875

Closed
4 tasks
kevlubkcm opened this issue Mar 14, 2019 · 17 comments · Fixed by #3954
Closed
4 tasks

CLI times out too quickly #3875

kevlubkcm opened this issue Mar 14, 2019 · 17 comments · Fixed by #3954
Assignees
Milestone

Comments

@kevlubkcm
Copy link
Contributor

Summary

A couple of times during cosmos mainnet launch, gaiacli would die with

Response:
ERROR: broadcast_tx_commit: Response error: RPC error -32603 - Internal error: Timed out waiting for tx to be included in a block

But the transaction was eventually successful. This seems very dangerous.
Not entirely sure what to do besides increase the timeout tho.
It isn't clear why a tx isn't being included.
Perhaps we can report at least if the tx is still sitting in peers' mempools?


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@joe-bowman
Copy link
Contributor

@kevlubkcm you are able to append --async to any tx, which will return a tx hash, which you can subsequently run gaiacli query tx <hash> to check if it has been committed in a block.

Another option would be making this async the default behaviour - but then you'd have to change it everywhere to be consistent. So perhaps better messaging is the key.

@kevlubkcm
Copy link
Contributor Author

kevlubkcm commented Mar 14, 2019 via email

@kevlubkcm
Copy link
Contributor Author

maybe also add a query for txs in mempool? something like

gaiacli query mempool --tags signer:<address>

just to quickly get all pending txs?

@faboweb faboweb added the REST label Mar 15, 2019
@faboweb
Copy link
Contributor

faboweb commented Mar 15, 2019

happens as well on REST inteface

@jackzampolin
Copy link
Member

Agree on the mempool query as well.

@jackzampolin jackzampolin added this to the v0.34.0 milestone Mar 16, 2019
@alexanderbez
Copy link
Contributor

What do you see as the actionable items here @jackzampolin? Make async the default or increase the timeout? Or both?

@gamarin2
Copy link
Contributor

I would increase the timeout

@jackzampolin
Copy link
Member

I think making timeout configurable would be the best. Also increasing the default. I don't think we should make txs async by default.

@jackzampolin
Copy link
Member

jackzampolin commented Mar 18, 2019

The mempool querying should be tracked by another issue.

@alexanderbez
Copy link
Contributor

alexanderbez commented Mar 20, 2019

So I took a look through github.com/tendermint/tendermint/rpc/client and I don't see any direct way to set or modify the write timeout of a Client. Any light you can shed on this @ebuchman? If my suspicion is correct, then TM will need to be updated first (e.g. a SetWriteTimeout on the HTTPClient interface).

@jackzampolin
Copy link
Member

Lets get this PR up

@ebuchman
Copy link
Member

We might want to avoid using the broadcast_tx_commit all together here to avoid this.

@kevlubkcm
Copy link
Contributor Author

What do you see as the actionable items here @jackzampolin? Make async the default or increase the timeout? Or both?

In addition to timeout increase, would be great if the command still returned a TxHash on failure so you can easily check later

@alexanderbez
Copy link
Contributor

alexanderbez commented Mar 20, 2019

Correct me if I'm wrong, but what I believe @ebuchman is saying is that we remove the support for sync (BroadcastTxSync) tx broadcasting all together.

The more I think about this, the more this makes sense to me. Increasing the timeout will not really solve the problem as the mempool can fill dramatically and once complex fee markets start to spring up into existence, it can be a while before a tx is included into a block.

So I vouch that we remove the sync option OR at the very least, make async the default and clearly stipulate that sync can timeout where the tx will eventually be included.

What do you think?

@kevlubkcm
Copy link
Contributor Author

that also makes sense to me

workflow looks more similar to say BTC/ETH (which i think would be familiar to most users) where you submit a tx and then wait to see it on a block explorer

@alexanderbez
Copy link
Contributor

Precisely! A flow with which most users are already familiar with anyway.

@ebuchman
Copy link
Member

it can be a while before a tx is included into a block.

Right.

In addition to timeout increase, would be great if the command still returned a TxHash on failure so you can easily check later

This would work too. We could just make the error message better and include the tx hash and instructions on what to do.

we remove the support for sync (BroadcastTxSync)

You mean BroadcastTxCommit ;). We want to use BroadcastTxSync instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants