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

Fix mpm integration-test hangs in rpc-fork mode, update test case on halley chain #3653

Merged
merged 6 commits into from
Aug 16, 2022

Conversation

pause125
Copy link
Collaborator

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Issue Number: N/A

What is the new behavior?

  • 解决 #3600 提到的 integration test 中,rpc fork 模式下 hang 住的问题。
  • 更新了基于 halley 测试网的 rpc fork 的测试 case。

Other information

@baichuan3 方便提供更多的 DAO 测试 case 吗? 我也可以一起测一下看看。

@codecov
Copy link

codecov bot commented Aug 13, 2022

Codecov Report

Merging #3653 (369c7de) into master (b2bc097) will increase coverage by 0.11%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3653      +/-   ##
==========================================
+ Coverage   28.65%   28.75%   +0.11%     
==========================================
  Files         591      591              
  Lines       50691    50686       -5     
  Branches    23903    23868      -35     
==========================================
+ Hits        14518    14570      +52     
- Misses      21928    21989      +61     
+ Partials    14245    14127     -118     
Flag Coverage Δ
unittests 28.75% <0.00%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
vm/natives/src/token.rs 26.67% <ø> (-4.44%) ⬇️
...starcoin-transactional-test-harness/src/context.rs 0.00% <0.00%> (ø)
...rcoin-transactional-test-harness/src/fork_state.rs 0.00% <0.00%> (ø)
vm/starcoin-transactional-test-harness/src/lib.rs 0.15% <ø> (ø)
...oin-transactional-test-harness/src/remote_state.rs 0.00% <0.00%> (ø)
vm/natives/src/account.rs 12.50% <0.00%> (-6.25%) ⬇️
network-rpc/core/src/lib.rs 20.00% <0.00%> (-6.00%) ⬇️
vm/types/src/token/token_code.rs 34.53% <0.00%> (-4.76%) ⬇️
sync/src/block_connector/test_write_block_chain.rs 21.71% <0.00%> (-4.65%) ⬇️
vm/types/src/transaction_metadata.rs 55.23% <0.00%> (-4.47%) ⬇️
... and 77 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3752f68...369c7de. Read the comment docs.

impl Drop for MockServer {
fn drop(&mut self) {
self.server_handle.abort();
Ok((
Copy link
Member

Choose a reason for hiding this comment

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

主要是这里导致卡住?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

之前用 tokio Runtime::spawn 启动 local server 的 server 端,然后 server 内部会用 futures::block_on 去执行 remote Client 的请求, 这样的用法会卡住。

server 内部用 Runtime::handle::block_on 去执行 remote Client 的请求,就会出现 nested Runtime 的问题。

现在是改成用 thread::spawn 启动 local server, server 内部用 Runtime::handle::block_on 其执行请求,就没问题了。

这里删掉 Drop 是因为启动 local server 的 thread::JoinHandle 没找到中断线程的方法,也就是没管 local server 的停机问题了。mpm 的场景下用应该也没啥问题,如果有解决方案当然更好。

@jolestar jolestar merged commit 75de44e into starcoinorg:master Aug 16, 2022
@baichuan3
Copy link

我来验证下这个问题,另外跑下另外的几个tesecase

@baichuan3
Copy link

验证在halley还是会hang住。
如果不使用halley网络,使用dev环境,测试正常。

测试case:https://github.com/starcoinorg/starcoin-framework/blob/dao_integration_test/integration-tests/daospace/callapi.move

@pause125
Copy link
Collaborator Author

验证在halley还是会hang住。 如果不使用halley网络,使用dev环境,测试正常。

测试case:https://github.com/starcoinorg/starcoin-framework/blob/dao_integration_test/integration-tests/daospace/callapi.move

我验证了一下这个 case, 不会卡住。可能测试时网络不好? 另外 fork 过程会从远程节点读取状态,可能会有点慢,可以多等一会看看(我跑这个 case 总共运行大约 35s)。

@jolestar
Copy link
Member

@baichuan3 可以更新一下 starcoin-framework 用的 mpm 版本,然后放 ci 里测。

@baichuan3
Copy link

确实成功了,就是比较慢,每次大概2~3分钟

@pause125
Copy link
Collaborator Author

@jolestar rpc fork 初始化的时候,会更新 state tree,导致需要读取数十个 state tree 节点, 这些节点都要通过 rpc 从远程获取,这个过程确实有点长,体验不是很好。

@jolestar
Copy link
Member

@jolestar rpc fork 初始化的时候,会更新 state tree,导致需要读取数十个 state tree 节点, 这些节点都要通过 rpc 从远程获取,这个过程确实有点长,体验不是很好。

dry run 的时候也有类似的问题。这种可能需要看看有没有办法批量获取。另外如果网络好的话其实问题应该也不大。

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.

4 participants