-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
TransactionScheduler: Consume Scheduler w/ PrioGraph #33612
Conversation
Codecov Report
@@ Coverage Diff @@
## master #33612 +/- ##
========================================
Coverage 81.7% 81.7%
========================================
Files 807 809 +2
Lines 218252 218606 +354
========================================
+ Hits 178438 178786 +348
- Misses 39814 39820 +6 |
In the example I gave above, the produced schedule isn't theoretically optimal, which would process the joining chains in parallel up to tx15. |
impl TopLevelId<Self> for TransactionPriorityId { | ||
fn id(&self) -> Self { | ||
*self | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this looks a bit odd, but there's a reason it's like this.
There's a custom prioritization function passed to the PrioGraph
.
That function produces anything which impls TopLevelId where Id
is the identifier for txs. That function can modify priority based on anything it likes...for example number of edges coming out of the node...but that is not currently being done, and it just uses the priority of this field directly for now.
Hoping github doesn't break from this 🙏 I don't think it shows anything unexpected, but it's nice to visually see and confirm some of our expectations:
I have a visualization tool that can spit these out...but mermaid/github didn't like when I tried graphing the entire slot of txs, so here's the top 100 priority txs received shortly before/during the slot: graph LR
Tx6894((Tx6894 - 175438))
Tx6890((Tx6890 - 100000))
Tx6891((Tx6891 - 100000))
Tx6892((Tx6892 - 100000))
Tx6893((Tx6893 - 100000))
Tx6888((Tx6888 - 55000))
Tx6889((Tx6889 - 55000))
Tx6883((Tx6883 - 50000))
Tx6884((Tx6884 - 50000))
Tx6885((Tx6885 - 50000))
Tx6886((Tx6886 - 50000))
Tx6887((Tx6887 - 50000))
Tx6869((Tx6869 - 20000))
Tx6870((Tx6870 - 20000))
Tx6871((Tx6871 - 20000))
Tx6872((Tx6872 - 20000))
Tx6873((Tx6873 - 20000))
Tx6874((Tx6874 - 20000))
Tx6875((Tx6875 - 20000))
Tx6876((Tx6876 - 20000))
Tx6877((Tx6877 - 20000))
Tx6878((Tx6878 - 20000))
Tx6879((Tx6879 - 20000))
Tx6880((Tx6880 - 20000))
Tx6881((Tx6881 - 20000))
Tx6882((Tx6882 - 20000))
Tx6868((Tx6868 - 14583))
Tx6867((Tx6867 - 12068))
Tx6864((Tx6864 - 11864))
Tx6865((Tx6865 - 11864))
Tx6866((Tx6866 - 11864))
Tx6863((Tx6863 - 11594))
Tx6861((Tx6861 - 11475))
Tx6862((Tx6862 - 11475))
Tx6857((Tx6857 - 10000))
Tx6858((Tx6858 - 10000))
Tx6859((Tx6859 - 10000))
Tx6860((Tx6860 - 10000))
Tx6855((Tx6855 - 8750))
Tx6856((Tx6856 - 8750))
Tx6852((Tx6852 - 8000))
Tx6853((Tx6853 - 8000))
Tx6854((Tx6854 - 8000))
Tx6840((Tx6840 - 2000))
Tx6841((Tx6841 - 2000))
Tx6842((Tx6842 - 2000))
Tx6843((Tx6843 - 2000))
Tx6844((Tx6844 - 2000))
Tx6845((Tx6845 - 2000))
Tx6846((Tx6846 - 2000))
Tx6847((Tx6847 - 2000))
Tx6848((Tx6848 - 2000))
Tx6849((Tx6849 - 2000))
Tx6850((Tx6850 - 2000))
Tx6851((Tx6851 - 2000))
Tx6836((Tx6836 - 1992))
Tx6837((Tx6837 - 1992))
Tx6838((Tx6838 - 1992))
Tx6839((Tx6839 - 1992))
Tx6831((Tx6831 - 1162))
Tx6832((Tx6832 - 1162))
Tx6833((Tx6833 - 1162))
Tx6834((Tx6834 - 1162))
Tx6835((Tx6835 - 1162))
Tx6830((Tx6830 - 1146))
Tx6816((Tx6816 - 1100))
Tx6817((Tx6817 - 1100))
Tx6818((Tx6818 - 1100))
Tx6819((Tx6819 - 1100))
Tx6820((Tx6820 - 1100))
Tx6821((Tx6821 - 1100))
Tx6822((Tx6822 - 1100))
Tx6823((Tx6823 - 1100))
Tx6824((Tx6824 - 1100))
Tx6825((Tx6825 - 1100))
Tx6826((Tx6826 - 1100))
Tx6827((Tx6827 - 1100))
Tx6828((Tx6828 - 1100))
Tx6829((Tx6829 - 1100))
Tx6811((Tx6811 - 1085))
Tx6812((Tx6812 - 1085))
Tx6813((Tx6813 - 1085))
Tx6814((Tx6814 - 1085))
Tx6815((Tx6815 - 1085))
Tx6810((Tx6810 - 1075))
Tx6809((Tx6809 - 1054))
Tx6808((Tx6808 - 1038))
Tx6790((Tx6790 - 1017))
Tx6791((Tx6791 - 1017))
Tx6792((Tx6792 - 1017))
Tx6793((Tx6793 - 1017))
Tx6794((Tx6794 - 1017))
Tx6795((Tx6795 - 1017))
Tx6796((Tx6796 - 1017))
Tx6797((Tx6797 - 1017))
Tx6798((Tx6798 - 1017))
Tx6799((Tx6799 - 1017))
Tx6800((Tx6800 - 1017))
Tx6801((Tx6801 - 1017))
Tx6802((Tx6802 - 1017))
Tx6894 --> Tx6883
Tx6894 --> Tx6868
Tx6894 --> Tx6855
Tx6890 --> Tx6892
Tx6892 --> Tx6893
Tx6883 --> Tx6884
Tx6884 --> Tx6885
Tx6885 --> Tx6886
Tx6886 --> Tx6887
Tx6872 --> Tx6857
Tx6869 --> Tx6870
Tx6869 --> Tx6873
Tx6870 --> Tx6871
Tx6871 --> Tx6875
Tx6871 --> Tx6873
Tx6873 --> Tx6874
Tx6874 --> Tx6876
Tx6874 --> Tx6875
Tx6875 --> Tx6876
Tx6875 --> Tx6878
Tx6876 --> Tx6877
Tx6877 --> Tx6878
Tx6877 --> Tx6879
Tx6878 --> Tx6879
Tx6878 --> Tx6881
Tx6879 --> Tx6880
Tx6880 --> Tx6881
Tx6880 --> Tx6882
Tx6881 --> Tx6882
Tx6868 --> Tx6867
Tx6868 --> Tx6863
Tx6867 --> Tx6855
Tx6867 --> Tx6864
Tx6864 --> Tx6865
Tx6865 --> Tx6866
Tx6866 --> Tx6863
Tx6866 --> Tx6861
Tx6863 --> Tx6861
Tx6861 --> Tx6862
Tx6862 --> Tx6855
Tx6858 --> Tx6860
Tx6858 --> Tx6859
Tx6859 --> Tx6860
Tx6855 --> Tx6856
Tx6852 --> Tx6854
Tx6840 --> Tx6841
Tx6840 --> Tx6843
Tx6841 --> Tx6842
Tx6842 --> Tx6845
Tx6842 --> Tx6843
Tx6843 --> Tx6844
Tx6843 --> Tx6846
Tx6843 --> Tx6845
Tx6844 --> Tx6847
Tx6844 --> Tx6845
Tx6845 --> Tx6848
Tx6845 --> Tx6846
Tx6846 --> Tx6848
Tx6846 --> Tx6847
Tx6846 --> Tx6849
Tx6847 --> Tx6848
Tx6847 --> Tx6850
Tx6848 --> Tx6851
Tx6848 --> Tx6849
Tx6849 --> Tx6850
Tx6849 --> Tx6851
Tx6850 --> Tx6851
Tx6836 --> Tx6837
Tx6837 --> Tx6838
Tx6838 --> Tx6839
Tx6839 --> Tx6831
Tx6839 --> Tx6811
Tx6839 --> Tx6830
Tx6831 --> Tx6832
Tx6832 --> Tx6833
Tx6833 --> Tx6834
Tx6834 --> Tx6835
Tx6835 --> Tx6830
Tx6835 --> Tx6811
Tx6830 --> Tx6809
Tx6830 --> Tx6811
Tx6816 --> Tx6817
Tx6816 --> Tx6820
Tx6817 --> Tx6818
Tx6818 --> Tx6819
Tx6819 --> Tx6820
Tx6819 --> Tx6821
Tx6820 --> Tx6823
Tx6820 --> Tx6821
Tx6821 --> Tx6822
Tx6822 --> Tx6824
Tx6822 --> Tx6823
Tx6823 --> Tx6824
Tx6823 --> Tx6825
Tx6824 --> Tx6825
Tx6824 --> Tx6826
Tx6825 --> Tx6828
Tx6825 --> Tx6826
Tx6826 --> Tx6827
Tx6827 --> Tx6828
Tx6827 --> Tx6829
Tx6828 --> Tx6829
Tx6811 --> Tx6812
Tx6812 --> Tx6813
Tx6813 --> Tx6814
Tx6814 --> Tx6815
Tx6815 --> Tx6809
Tx6815 --> Tx6810
Tx6810 --> Tx6809
Tx6809 --> Tx6808
Tx6790 --> Tx6791
Tx6791 --> Tx6792
Tx6792 --> Tx6793
Tx6793 --> Tx6794
Tx6794 --> Tx6795
Tx6795 --> Tx6796
Tx6796 --> Tx6797
Tx6797 --> Tx6798
Tx6798 --> Tx6799
Tx6799 --> Tx6800
Tx6800 --> Tx6801
Tx6801 --> Tx6802
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall looks good, believe look-ahead logic makes scheduling smarter. The mentioned performance trade-off is hard to judge from reading code. Would be good to measure and compare with real data.
core/src/banking_stage/transaction_scheduler/prio_graph_scheduler.rs
Outdated
Show resolved
Hide resolved
(&accounts[2], &[accounts[3].pubkey()], 1, 1), | ||
]); | ||
|
||
// high priority transactions [0, 1, 2, 3] do not conflict, and are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// high priority transactions [0, 1, 2, 3] do not conflict, and are | |
// The look-ahead window is intentionally shortened, high priority transactions [0, 1, 2, 3] do not conflict, and are |
core/src/banking_stage/transaction_scheduler/prio_graph_scheduler.rs
Outdated
Show resolved
Hide resolved
|
||
if num_scheduled >= MAX_TRANSACTIONS_PER_SCHEDULING_PASS { | ||
break; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: it'd be easier to read if this if
block is moved to the end of inner while
loop where it's value changed
core/src/banking_stage/transaction_scheduler/prio_graph_scheduler.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm - can land this scheduler, excited to see how it performs
Problem
Transaction scheduling in Banking Stage. The central scheduler MI approach had some short-comings in respecting priority that were not resolvable while mainitaining the performance.
Summary of Changes
A relatively simple scheduler that tracks in-flight transactions and account locks by thread.
This enables load-balancing and safe sending of transactions to threads that will process work sequentially without risk of inter-thread account lock conflicts*.
In addition, the scheduler uses a prio-graph (DAG w/ edges only to next-highest priority conflict) to look-ahead some number of transactions. Why is this useful?
Let's say we have 2 threads. It's very possible that
Tx6
andTx10
would be scheduled onto different threads, since they do not conflict. This would lead to transactions 15 and 16 being unschedulable until either transactions 14 or 9 (or both) are completed.However, the prio-graph tracks which independent chains of transactions eventually join within the lookup window, and so we know that
Tx6
andTx10
are ultimately part of the same chain, and should be scheduled onto the same thread.This example is smaller than realistic transaction load, in reality there are typically many more transactions.
* small caveat that there can still be inter-thread conflicts with the voting threads which have an independent scheduling for now.
Fixes #