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

Noting that process scheduling in ckb-vm is deterministic #445

Merged
merged 7 commits into from
Nov 18, 2024

Conversation

mohanson
Copy link
Contributor

@mohanson mohanson commented Oct 31, 2024

The developer may suspect that there will be randomness in the implementation of spawn, so this sentence is added intentionally.

@mohanson mohanson requested a review from a team as a code owner October 31, 2024 01:08
@mohanson mohanson requested a review from zhangsoledad October 31, 2024 01:08
@mohanson
Copy link
Contributor Author

cc @xxuejie @XuJiandong

@xxuejie
Copy link
Contributor

xxuejie commented Oct 31, 2024

I recommmend expanding this section a bit:

  • For each hardfork version, the process scheduling will be deterministic, any indeterminism will be treated as critical / security bugs that requires immediate intervention
  • However, based on real usage on chain, it is expected that future hardfork versions would improve the process scheduling workflow, hence making the behavior different across versions

Simply put, the scheduler included in CKB will be deterministic with respect to a particular hardfork version, and will likely change its behavior in different hardfork versions.

@XuJiandong
Copy link
Contributor

Provide detailed information about the deterministic scheduler, including:

  1. The criteria for selecting the next process to run when one process is blocked.
  2. The conditions under which deadlock can occur.
  3. The specific operations that can cause a process to become blocked.

@xxuejie
Copy link
Contributor

xxuejie commented Oct 31, 2024

Provide detailed information about the deterministic scheduler, including:

  1. The criteria for selecting the next process to run when one process is blocked.
  2. The conditions under which deadlock can occur.
  3. The specific operations that can cause a process to become blocked.

I'm not entirely sure here: are those information suitable for RFC, or separate documentations?

An analogy here, could be the details of the transaction pool, those are also highly tied to actual implementation, yet the RFCs does not fully describe those.

FYI: I'm not against adding those information, they certainly help, but it is worth discussing where they should fit.

@mohanson
Copy link
Contributor Author

I suggest putting the detailed working principle of the scheduler in a separate document.

@xxuejie
Copy link
Contributor

xxuejie commented Oct 31, 2024

A second thought: I think it makes sense to document the deadlock conditions for each hardfork versions here; but other details fit better in a separate document on Nervos docs.

@mohanson
Copy link
Contributor Author

A second thought: I think it makes sense to document the deadlock conditions for each hardfork versions here; but other details fit better in a separate document on Nervos docs.

There is already descriptions of when read/write/wait will cause deadlock

https://github.com/nervosnetwork/rfcs/blob/master/rfcs/0050-vm-syscalls-3/0050-vm-syscalls-3.md#write
https://github.com/nervosnetwork/rfcs/blob/master/rfcs/0050-vm-syscalls-3/0050-vm-syscalls-3.md#error-code

@xxuejie
Copy link
Contributor

xxuejie commented Oct 31, 2024

Yes but the words are quite vague:

If ckb-vm detects that all processes are blocked, ckb-vm will return a deadlock error.
It's possible for read/write/wait operations to wait for each other, leading to a deadlock state.

I think it makes sense to specifically define when all process will be blocked, and what it means for operations to wait for each other.

@mohanson
Copy link
Contributor Author

mohanson commented Nov 1, 2024

I added a separate section to describe deadlocks, please check @xxuejie @XuJiandong

XuJiandong
XuJiandong previously approved these changes Nov 14, 2024
rfcs/0050-vm-syscalls-3/0050-vm-syscalls-3.md Outdated Show resolved Hide resolved
rfcs/0050-vm-syscalls-3/0050-vm-syscalls-3.md Outdated Show resolved Hide resolved
rfcs/0050-vm-syscalls-3/0050-vm-syscalls-3.md Outdated Show resolved Hide resolved
rfcs/0050-vm-syscalls-3/0050-vm-syscalls-3.md Outdated Show resolved Hide resolved
rfcs/0050-vm-syscalls-3/0050-vm-syscalls-3.md Outdated Show resolved Hide resolved
rfcs/0050-vm-syscalls-3/0050-vm-syscalls-3.md Outdated Show resolved Hide resolved
@zhangsoledad zhangsoledad merged commit bd5d3ff into nervosnetwork:master Nov 18, 2024
1 check passed
@mohanson mohanson deleted the spawn-deterministic branch November 18, 2024 08:36
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