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

Fixes POSDI error #1873

Merged
merged 1 commit into from
Jan 16, 2024
Merged

Conversation

cryptowhizzard
Copy link
Contributor

POSDI index error fix for spade deals

POSDI index error fix for spade deals
@nonsense
Copy link
Member

@cryptowhizzard could you explain what the fix here is? Apart from DEBUG lines, I am not sure I see any changes to the current version of the code. Thanks!

@cryptowhizzard
Copy link
Contributor Author

cryptowhizzard commented Jan 16, 2024

That is a very good question. It's probably because we query the Filesize etc. I was debugging and looking why I got an EOF error running PODSI on some of the spade deals , but that is gone now and they are indexed.

So we are well on our way to finding the next bug , probably in RPC with lotus miner or something.

Anyway , I will ask Stuberman on slack to check if these deals do now pass and index with this patch and keep you updated. He experienced the same problem.

} else {
log.Debugw("File size", "size", fileSize)
}
_, err = file.Seek(0, io.SeekStart) // Return the file pointer to the start
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't we set the seeker back to the start of the segment index offset same as line 422? The current code would work for 32 GiB files but might be problematic for smaller deals.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am merging this to another branch so I can refactor the code.

Copy link
Member

Choose a reason for hiding this comment

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

yes, dsis is kind of important for the parseDataSegmentIndex usage below, 0 is not a good idea here

@LexLuthr LexLuthr changed the base branch from main to fix/podsi-eof January 16, 2024 15:45
@LexLuthr LexLuthr merged commit ed95973 into filecoin-project:fix/podsi-eof Jan 16, 2024
22 checks passed
LexLuthr added a commit that referenced this pull request Jan 25, 2024
* Fixes POSDI error (#1873)

POSDI index error fix for spade deals

* refactor the io.seek

---------

Co-authored-by: CryptoWhizzard <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants