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

app/charts: show epoch limit orders on depth chart #348

Merged
merged 2 commits into from
May 14, 2020

Conversation

buck54321
Copy link
Member

Shows epoch limit orders as a phantom layer on top of the order book. Differentiating is important here, because the epoch orders may or may not have processed by the time a placed order matches, so the epoch layer is not as likely to be matchable.

Updates to the test app server to more realistically handle epochs.

image

@@ -34,7 +34,7 @@ type BookFeed struct {
// feed when it's no longer being used.
func NewBookFeed(close func(feed *BookFeed)) *BookFeed {
return &BookFeed{
C: make(chan *BookUpdate, 1),
C: make(chan *BookUpdate, 256),
Copy link
Member

Choose a reason for hiding this comment

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

Did you experience a deadlock on a send or is this a performance consideration?

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops. Never submitted my self-review.

@@ -34,7 +34,7 @@ type BookFeed struct {
// feed when it's no longer being used.
func NewBookFeed(close func(feed *BookFeed)) *BookFeed {
return &BookFeed{
C: make(chan *BookUpdate, 1),
C: make(chan *BookUpdate, 256),
Copy link
Member Author

Choose a reason for hiding this comment

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

I ran into this in the test, and I think it's a risk during normal operations too, since the server sends updates for newly booked orders in rapid succession after matching.

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem I'm trying to avoid is at

select {
case feed.C <- u:
default:
log.Errorf("closing blocking book update channel")
delete(b.feeds, fid)
}

but it raises the question of whether that's the right pattern.

Copy link
Member

@chappjc chappjc May 12, 2020

Choose a reason for hiding this comment

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

I was battling with a similar challenge in #287 with sends on the orderRouter channel, resolved in commit edbf282. Ideally the channel buffer should only be a performance consideration (at least beyond a buffer of 1), and things should work if the buffer fills up. It's tough to be sure, but in the market's case it was a matter of proper synchronization between the sender and receiver so that the sender can guarantee there's a receiver on send. I'll give a proper review of this PR asap.

Comment on lines 298 to +299
switch {
case r < 0.33:
// Epoch order
trySend(&core.BookUpdate{
Action: msgjson.EpochOrderRoute,
Order: randomOrder(rand.Float32() < 0.5, c.maxQty, c.midGap, 0.05*c.midGap, true),
})
case r < 0.76:
case r < 0.80:
Copy link
Member Author

Choose a reason for hiding this comment

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

All new orders are being sent as epoch orders.

})
}
c.trySend(epochOrder)
c.epochOrders = append(c.epochOrders, epochOrder)
Copy link
Member Author

Choose a reason for hiding this comment

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

Epoch orders are cached for "matching".

@@ -34,7 +34,7 @@ type BookFeed struct {
// feed when it's no longer being used.
func NewBookFeed(close func(feed *BookFeed)) *BookFeed {
return &BookFeed{
C: make(chan *BookUpdate, 1),
C: make(chan *BookUpdate, 256),
Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops. Never submitted my self-review.

@chappjc chappjc self-requested a review May 12, 2020 22:20
Copy link
Member

@chappjc chappjc left a comment

Choose a reason for hiding this comment

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

I'm good with this. If you could rebase I think that would fix the checkout/v1 issue.

Shows epoch limit orders as a phantom layer on top of the order book.
Differentiating is important here, because the epoch orders may or
may not have processed by the time a placed order matches, so the
epoch layer is not as likely to be matchable.
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.

2 participants