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

FEATURE: [max] new method for depth buffer to set snapshot #1835

Merged
merged 1 commit into from
Nov 20, 2024

Conversation

kbearXD
Copy link
Collaborator

@kbearXD kbearXD commented Nov 19, 2024

No description provided.

@bbgokarma-bot
Copy link

Welcome back! @kbearXD, This pull request may get 221 BBG.

Copy link

codecov bot commented Nov 19, 2024

Codecov Report

Attention: Patch coverage is 0% with 22 lines in your changes missing coverage. Please review.

Project coverage is 23.21%. Comparing base (5b3f8b5) to head (dbd645a).
Report is 152 commits behind head on main.

Files with missing lines Patch % Lines
pkg/exchange/max/stream.go 0.00% 22 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1835      +/-   ##
==========================================
+ Coverage   23.08%   23.21%   +0.13%     
==========================================
  Files         625      636      +11     
  Lines       46942    48087    +1145     
==========================================
+ Hits        10837    11165     +328     
- Misses      35307    36113     +806     
- Partials      798      809      +11     
Files with missing lines Coverage Δ
pkg/exchange/max/stream.go 0.00% <0.00%> (ø)

... and 9 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

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

---- 🚨 Try these New Features:

@kbearXD kbearXD force-pushed the kbearXD/max/not-use-snapshot-event branch from dbd645a to cbd2876 Compare November 20, 2024 03:32
@bbgokarma-bot
Copy link

Re-estimated karma: this pull request may get 261 BBG

@kbearXD kbearXD requested review from c9s and ycdesu November 20, 2024 03:32
@kbearXD kbearXD changed the title FIX: not use snapshot event on max orderbook FEATURE: [max] new method for depth buffer to set snapshot Nov 20, 2024
}

// clean the buffer since we already set the snapshot
b.buffer = nil
Copy link
Owner

Choose a reason for hiding this comment

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

i guess we can still keep the updates?

b.mu.Lock()

if b.finalUpdateID >= finalUpdateID {
return nil
Copy link
Owner

Choose a reason for hiding this comment

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

unlock before return?

Asks: e.Asks,
}, e.FirstUpdateID, e.LastUpdateID); err != nil {
log.WithError(err).Errorf("found missing %s update event", e.Market)
if e.Event == "snapshot" {
Copy link
Owner

Choose a reason for hiding this comment

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

do we have a constant for this? in the maxapi/v3 package.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, we bypass this to book event.

@@ -75,6 +75,34 @@ func (b *Buffer) Reset() {
b.mu.Unlock()
}

func (b *Buffer) SetSnapshot(snapshot types.SliceOrderBook, firstUpdateID int64, finalArgs ...int64) error {
Copy link
Owner

Choose a reason for hiding this comment

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

make it private method?

@kbearXD kbearXD force-pushed the kbearXD/max/not-use-snapshot-event branch from cbd2876 to 93f68bc Compare November 20, 2024 05:51
@bbgokarma-bot
Copy link

Re-estimated karma: this pull request may get 326 BBG

@kbearXD kbearXD merged commit 505decc into main Nov 20, 2024
3 checks passed
@kbearXD kbearXD deleted the kbearXD/max/not-use-snapshot-event branch November 20, 2024 06:05
@bbgokarma-bot
Copy link

Hi @kbearXD,

Well done! 331 BBG has been sent to your polygon wallet. Please check the following tx:

https://polygonscan.com/tx/0x01e7cf3cb82d8d7cd3fa7c1ac2f399dbbd9fa49e98281698aae5776c8e6e04eb

Thank you for your contribution!

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