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

temp2 file should not be copied if final dir on same volume #8847

Closed
cross opened this issue Oct 17, 2021 Discussed in #8846 · 10 comments
Closed

temp2 file should not be copied if final dir on same volume #8847

cross opened this issue Oct 17, 2021 Discussed in #8846 · 10 comments
Labels
enhancement New feature or request Investigate - Eng Chia Engineering needs to be assigned to investigate stale-issue flagged as stale and will be closed in 7 days if not updated

Comments

@cross
Copy link
Contributor

cross commented Oct 17, 2021

Discussed in #8846

Originally posted by cross October 16, 2021
In the less common case where temp dir, or at least temp2, is on the same filesystem as the final dir, rename should be used instead of copy. Using chia 1.2.7 on linux, I started a plot as follows:

Starting plotting progress into temporary dirs: /data/chia/tmp and /data/chia/tmp
ID: [...]
Plot size is: 34
[...]
Final Directory is: /data/chia/plots/k34
Using 16 threads of stripe size 65536

On my system, /data is a large RAID array, so accepting the spindle contention, it's all on the same device. But at the end of this plot, I see:

Approximate working space used (without final file): 1176.562 GiB
Final File size: 429.934 GiB
Total time = 135175.666 seconds. CPU (145.320%) Mon Oct 11 02:07:45 2021
Copied final file from "/data/chia/tmp/plot-k34-2021-10-09-12-34-<keyval>.plot.2.tmp" to "/data/chia/plots/k34/plot-k34-2021-10-09-12-34-<keyval>.plot.2.tmp"
Copy time = 1952.410 seconds. CPU (37.220%) Mon Oct 11 02:40:39 2021
Removed temp2 file "/data/chia/tmp/plot-k34-2021-10-09-12-34-<keyval>.plot.2.tmp"? 1
Renamed final file from "/data/chia/plots/k34/plot-k34-2021-10-09-12-34-<keyval>.plot.2.tmp" to "/data/chia/plots/k34/plot-k34-2021-10-09-12-34-<keyval>.plot"

Unless I'm mistaken, the 1952 seconds spent copying the final file should've been a rename operation, which would also eliminate the removal of the temp2 file.

@cross
Copy link
Contributor Author

cross commented Oct 17, 2021

I'd be happy to contribute a fix for this, but I'm having trouble finding where this copying is happening. Searching the code for "Copied" or "final file" has unexpectedly failed me.

@loppefaaret loppefaaret added the enhancement New feature or request label Oct 18, 2021
@loppefaaret
Copy link

The current design is issuing a copy, if the folder is not the exact same for both tmp2 and destination, even if on the same volume - you could point tmp2 to the destination instead of a tmp folder to circumvent the final copy - it would then do a rename instead

Ive labelled is as an enhancement request

@cross
Copy link
Contributor Author

cross commented Oct 18, 2021

Thank you for the detail @loppefaaret Two questions, (1) is writing to tmp2 done any time other than writing what will be the final file? I could do as you suggest and set it to the final dir, but am curious if there is other writing to tmp2 during plotting, or just that final file.
(2) Where in the code is the decision made, and action initiated, to move (or rename if same dir) the final file? I couldn't find it.

@loppefaaret
Copy link

tmp2 is used for the resulting .plot file, it is build during phase 3 and indexed in phase 4 - no, no writing other than the resulting data takes place to tmp2

@cross
Copy link
Contributor Author

cross commented Oct 20, 2021

Thank you. If someone could direct me to where that existing decision is made, copy or rename (if in final directory already), I will work on a PR that will rename any plots that can be renamed instead of copied.

@loppefaaret
Copy link

please dont rip my head off if this is not the place, im not a dev, just support:

async def create_plots(

@cross
Copy link
Contributor Author

cross commented Oct 20, 2021

Thanks. Looks right. But, looking there, I see the actual logic I want to change is in chiapos. So, I'll take this up over there. Thanks.

@cross
Copy link
Contributor Author

cross commented Oct 21, 2021

Fix for this is in PR Chia-Network/chiapos#314 - if you know someone who can approve the workflows there @loppefaaret please do.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2021

This issue has not been updated in 14 days and is now flagged as stale. If this issue is still affecting you and in need of further review, please comment on it with an update to keep it from auto closing in 7 days.

@github-actions github-actions bot added the stale-issue flagged as stale and will be closed in 7 days if not updated label Nov 5, 2021
@paulhainsworth-chia paulhainsworth-chia added the Investigate - Eng Chia Engineering needs to be assigned to investigate label Nov 11, 2021
@github-actions
Copy link
Contributor

This issue was automatically closed because it has been flagged as stale, and subsequently passed 7 days with no further activity from the submitter or watchers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Investigate - Eng Chia Engineering needs to be assigned to investigate stale-issue flagged as stale and will be closed in 7 days if not updated
Projects
None yet
Development

No branches or pull requests

3 participants