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.

Sign up for GitHub

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

Jump to bottom

Eliminate inefficiency in markets logging #6895

Merged
merged 2 commits into from
Jul 29, 2021
Merged

Eliminate inefficiency in markets logging #6895

merged 2 commits into from
Jul 29, 2021

Conversation

hannahhoward
Copy link
Contributor

Goals

Whenever we have a data transfer event we currently read in the entire list of received CIDs off disk just to get their length.
We have a function that can return length without reading the CID list

Implementation

  • Use ReceivedCidsLen() rather than len(ReceivedCids)

@hannahhoward
fix(marketevents): use ReceivedCidsLen
f4fa4c8 
improve data transfer logger by not reading in entire cid lists
@hannahhoward hannahhoward requested a review from a team as a code owner July 28, 2021 00:12
@hannahhoward hannahhoward changed the title Eliminate large inefficiency in markets logging Eliminate inefficiency in markets logging Jul 28, 2021
@magik6k magik6k enabled auto-merge July 28, 2021 14:05
@magik6k
Copy link
Contributor

magik6k commented Jul 28, 2021 •
edited
Loading

(Will need to be ported / PRed to master as well)

raulk
raulk approved these changes Jul 28, 2021
View reviewed changes
Copy link
Member

@raulk raulk left a comment

Choose a reason for hiding this comment

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

This is already fixed in the m1 branches.

@jennijuju
Copy link
Member

this needs to be in master

@jennijuju jennijuju disabled auto-merge July 29, 2021 05:08
@jennijuju jennijuju merged commit c8b0f11 into release/v1.11.1 Jul 29, 2021
@jennijuju jennijuju deleted the fix/data-transfer-logging-received-cids branch July 29, 2021 05:09
This was referenced Jul 29, 2021
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@magik6k magik6k magik6k approved these changes

@Kubuxu Kubuxu Kubuxu approved these changes

@raulk raulk raulk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@hannahhoward @magik6k @jennijuju @raulk @Kubuxu