Skip to content

Conversation

@positiveblue
Copy link
Contributor

Quick (dirty) fix to ensure that we do not end up in an endless loop of batch rejections. One side effect is that we pollute the logs a bit more (issue #327).

#341 will fix this properly, by ensuring that shim creation after matching is atomic. Meanwhile this unblocks some of our traders.

Copy link
Contributor

@guggero guggero left a comment

Choose a reason for hiding this comment

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

LGTM, nice quick fix!


// Ensure that we do not have any registered shims for the orders
// in this batch. This is not supposed to happen but we have a bug.
if err = a.removeShims(batch); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps link to the PR where this will be fixed for good?
Also add a comment that removeShims will only return an error for serious problems (for example not being able to read orders from disk) and not if there is no pending shim (only a warning will be logged but no error returned).

@positiveblue positiveblue merged commit 1ec8849 into master Feb 25, 2022
@guggero guggero deleted the clean-old-order-shims branch May 5, 2022 15:25
positiveblue pushed a commit to positiveblue/pool that referenced this pull request Oct 11, 2022
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.

3 participants