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

Bump oasis-core version to 20.9 #36

Merged
merged 4 commits into from
Aug 17, 2020
Merged

Bump oasis-core version to 20.9 #36

merged 4 commits into from
Aug 17, 2020

Conversation

pro-wh
Copy link
Collaborator

@pro-wh pro-wh commented Aug 12, 2020

fixes #30

@pro-wh pro-wh requested a review from kostko August 12, 2020 21:48
@pro-wh
Copy link
Collaborator Author

pro-wh commented Aug 12, 2020

2020-08-12T21:53:09.4698910Z 2020/08/12 21:53:09 Post "http://localhost:8080/construction/submit": context deadline exceeded (Client.Timeout exceeded while awaiting headers): unable to broadcast transaction: unable to broadcast transaction 25e7097b2a1a7cd036767787386b45c4a47cf0e5a622b979b12b33c2a64cbac3

and then it fails all the retries because the tx already exists

2020-08-12T21:53:09.7361701Z ts=2020-08-12T21:53:09.735967044Z level=error module=grpc/client caller=grpc.go:242 msg="request failed" method=/oasis-core.Consensus/SubmitTx req_seq=580 rsp=null err="rpc error: code = Unknown desc = tendermint: failed to submit to local mempool: tx already exists in cache"
2020-08-12T21:53:09.7362913Z ts=2020-08-12T21:53:09.736007643Z level=error module=services/construction caller=construction.go:129 msg="ConstructionSubmit: SubmitTx failed" err="rpc error: code = Unknown desc = tendermint: failed to submit to local mempool: tx already exists in cache"
2020-08-12T21:53:09.7364220Z 2020/08/12 21:53:09 {Code:15 Message:unable to submit transaction Retriable:false Details:map[]}: unable to broadcast transaction: unable to broadcast transaction 25e7097b2a1a7cd036767787386b45c4a47cf0e5a622b979b12b33c2a64cbac3

@kostko
Copy link
Member

kostko commented Aug 13, 2020

Hm, why is the deadline suddenly exceeded, is the transaction not included in a block for some reason?

maybe we should get going on that submit-doesn't-block-for-confirmation issue #24

Yeah that should be a trivial change, just change the method name.

maybe we should hook up "transaction already in mempool" to return success

Yeah this would probably be in line with Rosetta semantics (no error if transaction is added to mempool). We could add a proper error type for this on the Oasis Core side so that you wouldn't need to match strings (we support transporting actual error codes across RPC boundaries).

oasis-client/oasis-client.go Outdated Show resolved Hide resolved
@pro-wh
Copy link
Collaborator Author

pro-wh commented Aug 13, 2020

Yeah that should be a trivial change, just change the method name.

what's the method for the non-wait submit?

@pro-wh
Copy link
Collaborator Author

pro-wh commented Aug 13, 2020

Hm, why is the deadline suddenly exceeded, is the transaction not included in a block for some reason?

I had the tests succeed locally, so maybe it really is the timing. did 20.9 have a performance regression?

@abukosek
Copy link
Contributor

what's the method for the non-wait submit?

SubmitTxNoWait -- it's part of the light client API :)

@pro-wh
Copy link
Collaborator Author

pro-wh commented Aug 13, 2020

what's the method for the non-wait submit?

oh it's SubmitTxNoWait. of course. I hadn't looked in those embedded structs

@pro-wh
Copy link
Collaborator Author

pro-wh commented Aug 13, 2020

2020-08-13T19:48:55.9575866Z Check failed: broadcast failed for transaction 0841f461374fc6fb10ea5e9783af92e23c92d446960f00c47d218206568c2087 with intent [
...

well this error's just silly

https://github.com/coinbase/rosetta-cli/blob/v0.4.0/internal/processor/broadcast_storage_handler.go#L82-L96

the CLI test is just killing itself by going over its broadcast limit

edit: oh nvm the limit is on retries

@pro-wh
Copy link
Collaborator Author

pro-wh commented Aug 13, 2020

ok what's confusing me now is that in https://github.com/oasisprotocol/oasis-core-rosetta-gateway/runs/977917540

2020-08-12T21:53:19.4740394Z Transaction Created: 25e7097b2a1a7cd036767787386b45c4a47cf0e5a622b979b12b33c2a64cbac3

shows up after several lines of

2020-08-12T21:53:09.7364220Z 2020/08/12 21:53:09 {Code:15 Message:unable to submit transaction Retriable:false Details:map[]}: unable to broadcast transaction: unable to broadcast transaction 25e7097b2a1a7cd036767787386b45c4a47cf0e5a622b979b12b33c2a64cbac3

but in the code https://github.com/coinbase/rosetta-cli/blob/v0.4.0/internal/constructor/constructor.go#L965-L968 it's supposed to log before it tries to broadcast

@pro-wh
Copy link
Collaborator Author

pro-wh commented Aug 13, 2020

eh, maybe differences in flushing between log.Printf and color.Magenta

@pro-wh
Copy link
Collaborator Author

pro-wh commented Aug 13, 2020

no, weirder. those submissions must be coming from a different goroutine. I added some debugging in rosetta-cli, and it's being broadcast as part of addBlockCommitWorker

@pro-wh
Copy link
Collaborator Author

pro-wh commented Aug 13, 2020

ok I give up on this bug. I'm going to investigate the test failure in #35, which comes after the switch to non-block submission

@pro-wh
Copy link
Collaborator Author

pro-wh commented Aug 13, 2020

naw, you know what? maybe it is the flushing. those lines at the end

[STATS] Transactions Confirmed: 0 (Created: 0, In Progress: 0, Stale: 0, Failed: 0) Addresses Created: 0
Transaction Created: 1c78b5a23bf2cfd3695ad8c740f7347a8723212ef643786f79636aa963808166
  oasis1qzzd6khm3acqskpxlk9vd5044cmmcce78y5l6000 -- 92.584656794 ROSE --> oasis1qqjlg4rv2ggt3n8q5yp54hx35tjxhhjp7cxxheau
[STATS] Transactions Confirmed: 0 (Created: 1, In Progress: 1, Stale: 1, Failed: 0) Addresses Created: 1
Transaction Created: 7f028d077475b6a78ee018cfe46d7d7ca02deeeb41116a4e136142dfed60a8a4
  oasis1qqjlg4rv2ggt3n8q5yp54hx35tjxhhjp7cxxheau -- 75.600617812 ROSE --> oasis1qpaakr8mn0k2j387unyh7tkr8s8mkp6ekg046rc8
[STATS] Transactions Confirmed: 1 (Created: 1, In Progress: 1, Stale: 1, Failed: 0) Addresses Created: 2
Transaction Created: 0841f461374fc6fb10ea5e9783af92e23c92d446960f00c47d218206568c2087
  oasis1qzzd6khm3acqskpxlk9vd5044cmmcce78y5l6000 -- 3.234129515 ROSE --> oasis1qzarmevhezdr6ceezc6tg0adrk6w3swuc5t2222r
[STATS] Transactions Confirmed: 1 (Created: 2, In Progress: 2, Stale: 1, Failed: 0) Addresses Created: 3
[STATS] Transactions Confirmed: 2 (Created: 3, In Progress: 1, Stale: 4, Failed: 0) Addresses Created: 3
Transaction Created: 6991139bdb67529d53dd1723ae3d3792b51d7db658dc4cd0e05c77494b3dfed4
  oasis1qpaakr8mn0k2j387unyh7tkr8s8mkp6ekg046rc8 -- 61.748548346 ROSE --> oasis1qqjlg4rv2ggt3n8q5yp54hx35tjxhhjp7cxxheau
[STATS] Transactions Confirmed: 2 (Created: 3, In Progress: 2, Stale: 4, Failed: 0) Addresses Created: 3
[STATS] Transactions Confirmed: 2 (Created: 4, In Progress: 1, Stale: 5, Failed: 1) Addresses Created: 3

are all normally colored text.

@pro-wh
Copy link
Collaborator Author

pro-wh commented Aug 14, 2020

didn't help to wrap rosetta-cli in stdbuf -oL. are some of these messages coming out over stderr or something?

@pro-wh
Copy link
Collaborator Author

pro-wh commented Aug 14, 2020

oh, all of that above the normally-colored lines was from the gateway. it's awk then

@pro-wh pro-wh force-pushed the pro-wh/feature/corebump branch from 94817e9 to b0cf662 Compare August 14, 2020 00:50
@pro-wh
Copy link
Collaborator Author

pro-wh commented Aug 14, 2020

correction: no it was not all from the gateway. there was another logging system in place, going to stderr

@pro-wh pro-wh force-pushed the pro-wh/feature/corebump branch from 0a10ca4 to c1db72f Compare August 17, 2020 18:46
@pro-wh
Copy link
Collaborator Author

pro-wh commented Aug 17, 2020

ok there's new advice on this blocks-produced-very-fast issue #38 (comment)

let's see if we can go without 0a10ca4

@pro-wh
Copy link
Collaborator Author

pro-wh commented Aug 17, 2020

@pro-wh pro-wh merged commit b3f8c83 into master Aug 17, 2020
@pro-wh pro-wh deleted the pro-wh/feature/corebump branch August 17, 2020 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use Oasis Core 20.9
3 participants