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

Continuous Websocket Message Reading on Readiness Event #2852

Conversation

Jayanring
Copy link
Contributor

  • Read message repeatly when receive a readiness event

Copy link

linear bot commented Jul 1, 2024

Copy link
Collaborator

@Kailai-Wang Kailai-Wang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks!

Should identity worker (in tee-worker/) be changed too?

@Kailai-Wang Kailai-Wang requested review from kziemianek and a team July 1, 2024 19:03
@BillyWooo
Copy link
Collaborator

BillyWooo commented Jul 1, 2024

Should identity worker (in tee-worker/) be changed too?

I think it should.

Copy link
Member

@kziemianek kziemianek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ws server is single threaded so draining connection will block it until it's finished.
This might be good enough for bitacross-worker but not for tee-worker.

However Poll::Level is used so mio events should be emitted as long as there is data in buffer and draining strategy should be used only in case of Poll::Edge. Currenlty we don't know why it behaves this way and this looks only like a workaround.

@Kailai-Wang
Copy link
Collaborator

There could be two possible reasons:

  • the tungstenite read_message somehow reads all messages in one go and/or clear the socket reading state, so that mio thinks there's nothing left
  • the re-register of mio is buggy (delay? race condition?) and doesn't trigger events even if there're still remaining data that were sent quickly after the first message

Either way - can this be reproduced?
If so we could try to dig further and find out when the first message is handled, if there's anything left in the socket just before re-registering

@Kailai-Wang
Copy link
Collaborator

Had a discussion with Kasper - we'll keep this workaround only for bitacross-worker.

Root cause is not exaclty known but it's likely the old mio crate is buggy - we see messages being kept in the read buffer in the socket but no mio event is triggered.

Jayanring added 2 commits July 3, 2024 08:47
…n-handling-concurrent-requests' into p-894-bitacross-worker-stuck-when-handling-concurrent-requests
@Jayanring Jayanring requested a review from BillyWooo July 3, 2024 09:04
@Kailai-Wang Kailai-Wang merged commit 6691a68 into dev Jul 3, 2024
33 checks passed
@Kailai-Wang Kailai-Wang deleted the p-894-bitacross-worker-stuck-when-handling-concurrent-requests branch July 3, 2024 12:10
wangminqi added a commit that referenced this pull request Jul 12, 2024
* dockerfile fix (#2853)

* client-api: package update for publishing (#2850)


---------

Signed-off-by: Jonathan Alvarez <jonathan@litentry.com>
Co-authored-by: 0xverin <104152026+0xverin@users.noreply.github.com>

* close ws connection in case of BrokenPipe error (#2854)

* fix: add developer committee to basefilter (#2855)

* Continuous Websocket Message Reading on Readiness Event (#2852)

* ws connection read messages fix

* fix fmt

* optim handling

* close ws when read error

* optimize error handling

---------

Co-authored-by: Kasper Ziemianek <kasper.ziemianek@gmail.com>
Co-authored-by: Jayanring <junjie@liteng.io>

* Bump syn from 2.0.66 to 2.0.68 (#2839)

Bumps [syn](https://github.com/dtolnay/syn) from 2.0.66 to 2.0.68.
- [Release notes](https://github.com/dtolnay/syn/releases)
- [Commits](dtolnay/syn@2.0.66...2.0.68)

---
updated-dependencies:
- dependency-name: syn
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Adjust logging level and messages (#2857)

* adjust log level

* use trace lvl

* more log

* fix clippy

* Deprecate `ScheduledEnclave` and introduce `AuthorizedEnclave` (#2856)

* add AuthorizedEnclave

* adjust tests

* remove old scheduled enclave

* bump version

* remove indirect call

* Use new keys from env setting (#2858)

* Add teebag and bitacross to litentry runtime (#2859)

* Add teebag and bitacross to litentry runtime

* fix compile

* P-872 basic tests for contracts (#2845)

* Flattening contracts

* fix omports

* init tests

* add compile.json

* fix params

* fix assertionId

* try encryptWithTeeShieldingKey

* fix encryptedSecrets

* add ci shell

* fmt

* add lit-tee-vc-contracts-test

* rename

* chmod +x

* source /root/.bashrc

* install git

* remove sudo

* fix shell scripts

* ls

* ls one more

* ls one more

* add volumes

* cp contracts&&compile

* remove hardcode json

* fix entrypoint

* fix compilation

* try to fix assertionId

* testing:cli encrypt

* fmt

* testing:cli response

* testing:create assertionId with secretsEncryptedByCli

* check:Is cli encrypt the same as ts encrypt

* add sleep

* update README

* merge assertion tests shell

* fix docker yml

* identity-linking via cli

* remove unused code

* add developerCommittee

* rename scripts name

* add alice to developerCommittee

* fix copy-contracts

* identity-linking using di call

* feat: P-860 add logging support for dynamic contract (#2848)

* feat: P-860 add logging support for dynamic contract

* fix: change vc_logs to optional in RequestVCResult, default log response in http precompile contract

* remove default log response in http precompile contract

* fix: update ts definition for dynamic assertion

* fix clippy

* fix: change type of return_log to bool, using Precompiles instead of thread_local variable DYNAMIC_ASSERTION_LOGS, only include vc_logs in rpc response

* add the log to vc_logs in all json_utils precompile contracts, do not return vc_logs if stf is true when using CLI

* return vc_logs if stf is true

* add logs in dynamic contract A20

---------

Co-authored-by: higherordertech <higherordertech>

* reuse websocket connection for ceremony rounds (#2861)

* Adding  force-migrate-shard flag to run worker command (#2862)

* fix dynamic assertion integration test (#2864)

* Add remove_vault in pallet-bitacross (#2863)

* Add remove_vault

* Do not pay fees for now

* Improve shard migration (#2866)

* removing force-migrate-shard and improving migrate-shard

* improving mrenclave command

* fixing ci

* putting back comment

* cleaning up mock

* update cli doc

---------

Co-authored-by: Kailai Wang <Kailai.Wang@hotmail.com>

* Disable docker build record upload (#2867)

* feat: add DelegatorState CandidateInfo migration

* debug: fmt

* chore

* chore

* chore

* chore

* feat: add a parachain event for completion of contract sync (#2870)

* feat: add callback extrinsic for extrinsic

* fix: add TEE Origin

* fix: add evm_assertions metadata

* fix: fix test error

* refactor: fix clippy

* chore

* chore

* chore

* chore

* chore

* chore

* feat: P-914 implemented NFT token holder VC for MFAN on Polygon (#2877)

Co-authored-by: higherordertech <higherordertech>

* Disable issue creation GHA (#2881)

* fix: request-vc in cli with ii and stf fails to decode (#2882)

* fix: request-vc in cli with ii and stf fails to decode

* refactor: remove unused import

* chore

* feat: add ScheduledRequest

* feat: add TopDelegations

* chore

* chore

* chore

* chore

* chore: allow clippy type complexity

* chore: add bitacross try-runtime

* An and tuna holding amount vc (#2884)

* feat: P-915 implemented token holding amount for AN

* feat: P-915 implemented token holding amount for TUNA

* fix: updated TokenHoldingAmount credentials schema

---------

Co-authored-by: higherordertech <higherordertech>

* chore

* feat: add Total storage

* Update dependabot deps (#2880)

Co-authored-by: BillyWooo <thedreamofbilly@gmail.com>

* Support DCAP RA (#2869)

* remove println

* consider prod too

* add RA_METHOD

* temporarily ignore tcb check

* lenient tcb level

* use Intel for default dcap provider

* same change for bc

* patch Makefile

---------

Co-authored-by: BillyWooo <thedreamofbilly@gmail.com>

* chore

* feat: add CandidatePool, DelayedPayouts, Staked

* chore

* chore

* chore

* chore

* chore

---------

Signed-off-by: Jonathan Alvarez <jonathan@litentry.com>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Mi1kTea <73102617+m1iktea@users.noreply.github.com>
Co-authored-by: Jonathan Alvarez <jonathan@litentry.com>
Co-authored-by: 0xverin <104152026+0xverin@users.noreply.github.com>
Co-authored-by: Kasper Ziemianek <kasper.ziemianek@gmail.com>
Co-authored-by: Faisal Ahmed <42486737+felixfaisal@users.noreply.github.com>
Co-authored-by: Jayanring <81245047+Jayanring@users.noreply.github.com>
Co-authored-by: Jayanring <junjie@liteng.io>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Kai <7630809+Kailai-Wang@users.noreply.github.com>
Co-authored-by: will.li <120463031+higherordertech@users.noreply.github.com>
Co-authored-by: Francisco Silva <franjs.francisco@gmail.com>
Co-authored-by: Kailai Wang <Kailai.Wang@hotmail.com>
Co-authored-by: BillyWooo <thedreamofbilly@gmail.com>
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.

None yet

4 participants