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

PPM writes directly to pup_rw #3115

Closed
dimkr opened this issue May 26, 2022 · 17 comments
Closed

PPM writes directly to pup_rw #3115

dimkr opened this issue May 26, 2022 · 17 comments

Comments

@dimkr
Copy link
Contributor

dimkr commented May 26, 2022

This is dangerous (the behavior of overlay is undefined), and can cause problems (like directory symlinks that don't get dereferenced by cp).

Why not just write files to /?

@zpunorm22
Copy link

zpunorm22 commented May 26, 2022

Just as a test I attempted to install Steam on a newly built Puppy (based on Ubuntu 22.04 or Jammy Jellyfish), but that seems to remove essential files from the system (like /bin/bash). Possible bug that may be related?

@dimkr
Copy link
Contributor Author

dimkr commented May 27, 2022

@zpunorm22 Using PPM?

@zpunorm22
Copy link

zpunorm22 commented May 27, 2022

I tried installing it via PPM, Quickpet (from fossapup64), and directly using petget. Same thing happens.

@dimkr
Copy link
Contributor Author

dimkr commented May 27, 2022

apt install steam works just fine in https://forum.puppylinux.com/viewtopic.php?p=58153 (built from testing + #2963).

It's not surprising that PPM fails to install Steam, because PPM doesn't distinguish between 32-bit and 64-bit packages (i.e. a 32-bit dependency library replaces the 64-bit library, although they can coexist).

@zpunorm22
Copy link

zpunorm22 commented May 27, 2022

Well, what if apt is not included in the build, like in the build of the current testing branch? The only way to install apt is via PPM.
And yes, I tried that too. It also breaks the system the same way. PPM is broken.

@dimkr
Copy link
Contributor Author

dimkr commented May 27, 2022

Well, what if apt is not included in the build, like in the build of the current testing branch?

It is included in the build, if you ./3builddistro release.

And yes, PPM has many issues, and the thing you're talking about has nothing to do with this issue (#3115).

@mavrothal
Copy link
Contributor

I think is a development relic from the times that layer file systems where at early stages the storage limited and PPM was used for puppy building, but you have to ask BK for that.
Would make PPM code much simpler without the hunt for the proper save layer.
Will only needs to look for whiteouts there really.

@dimkr
Copy link
Contributor Author

dimkr commented May 28, 2022

Would make PPM code much simpler without the hunt for the proper save layer.
Will only needs to look for whiteouts there really.

I'm talking about this line in /usr/local/petget/installpkg.sh:

tar -x --force-local --strip=2 --directory=${DIRECTSAVEPATH}/ -f ${tarball}

Changing DIRECTSAVEPATH to / won't simplify the code, and I don't understand why it's important not to use DIRECTSAVEPATH=/. Once DIRECTSAVEPATH=/, aufs itself will take care of cleaning up whiteouts.

@gyrog
Copy link
Contributor

gyrog commented May 28, 2022

Installing to "/" is a long overdue mod to PPM.
Then the code for finding the ${DIRECTSAVEPATH} becomes obselete.
And so does the code mucking about with whiteout files.

This is exactly the problem PPM had when Puppy used to have some libs as symbolic links defined in the 'puppy_....sfs', not in the ${DIRECTSAVEPATH}.
Neither PPM nor the install package was aware of the symbolic link so created a real directory in the ${DIRECTSAVEPATH}.

@gyrog
Copy link
Contributor

gyrog commented May 30, 2022

Changing DIRECTSAVEPATH to / won't simplify the code, and I don't understand why it's important not to use DIRECTSAVEPATH=/. Once DIRECTSAVEPATH=/, aufs itself will take care of cleaning up whiteouts.

Because while it might work as a "quick fix", it leaves whiteout processing code and DIRECTSAVEPATH finding code that is no longer needed.

@rizalmart
Copy link
Contributor

rizalmart commented May 30, 2022

@dimkr @mavrothal

Barry implements odd numbered pupmodes were the tmpfs was on the topmost and the save file was on the bottom of tmpfs layer. It caches the modified or created files then flushing to save file periodically or upon shutdown. The advantage was preventing frequent writes to SSD/Flash storage thus prolonging the life of SSD or Flash memory. Thats why DIRECTSAVEPATH was vital because. Upon installation of packages, it goes directly too save file not on / because if fills the writable layer quickly if odd numbered pupmode was implemented

@mavrothal
Copy link
Contributor

Thats why DIRECTSAVEPATH was vital because

Sounds right!
However, today USB sticks are fairly reliable and memory is abundant, particularly with the zram implementation.
If in systems with limited /temp size and in the relevant PUPMODE, snapmerge could be called to relieve the pressure

@dimkr
Copy link
Contributor Author

dimkr commented May 31, 2022

The advantage was preventing frequent writes to SSD/Flash storage thus prolonging the life of SSD or Flash memory

Actually, I don't know if this is true. I'm not sure if PUPMODE 13 reduces writing: for example, the browser cache is huge (hundreds of MBs) and changes often. Instead of writing only the delta to pup_ro1 (flushing just the changes), the whole thing gets flushed to pup_ro1 every time. Instead of writing few KBs or MBs often, snapmergepuppy writes hundreds of MBs on every save.

However, today USB sticks are fairly reliable and memory is abundant, particularly with the zram implementation.
If in systems with limited /temp size and in the relevant PUPMODE, snapmerge could be called to relieve the pressure

👍🏿

@rizalmart
Copy link
Contributor

rizalmart commented May 31, 2022

Actually, I don't know if this is true. I'm not sure if PUPMODE 13 reduces writing: for example, the browser cache is huge (hundreds of MBs) and changes often. .

@dimkr
Prolonging the life of SSD is not only by reduced writing cycle but also by increased writing duration on SSD

Instead of writing only the delta to pup_ro1 (flushing just the changes), the whole thing gets flushed to pup_ro1 every time. Instead of writing few KBs or MBs often, snapmergepuppy writes hundreds of MBs on every save

That was the apparent flaw. Just modified the snapmegepuppy to exclude internet cache files except session and cookies. The internet cache files located at $HOME/.cache

@dimkr
Copy link
Contributor Author

dimkr commented Jun 1, 2022

That was the apparent flaw. Just modified the snapmegepuppy to exclude internet cache files except session and cookies. The internet cache files located at $HOME/.cache

The browser cache must be retained. Otherwise, browsing will be slower, and Puppy will eat your data plan. #3129 is a really bad idea IMO.

However, the browser doesn't write hundreds of MBs on every change of the cache. The browser creates new files or modifies portions of existing files (for example, portions of a sqlite database). For example, a single change in a database can change only a 4K of portion of the file. But snapmergepuppy will replace a 100 MB database in pup_ro1 with a 100 MB file (= 100 MB of I/O), instead of modifying the existing file (writing just the 4K delta to the right offset).

EDIT: #3129 will have more consequences - for example, it will break ccache (the whole point of using ccache is keeping the cache between builds).

@rizalmart
Copy link
Contributor

rizalmart commented Jun 2, 2022

@dimkr

The browser cache must be retained. Otherwise, browsing will be slower, and Puppy will eat your data plan. #3129 is a really bad idea IMO.

The browser cache was still in pup_rw tmpfs layer. It will not easily ate data plan. Those browser cache might be deleted if pup_rw tmpfs free memory was low.

However, the browser doesn't write hundreds of MBs on every change of the cache.

Yeah but overtime it build huge disk space consumption

The browser creates new files or modifies portions of existing files (for example, portions of a sqlite database). For example, a single change in a database can change only a 4K of portion of the file. But snapmergepuppy will replace a 100 MB database in pup_ro1 with a 100 MB file (= 100 MB of I/O), instead of modifying the existing file (writing just the 4K delta to the right offset).

FIrst of all that's the nature of AUFS. The full modified files was always at topmost writable layer not a differential ones.

a single change in a database can change only a 4K of portion of the file. But snapmergepuppy will replace a 100 MB database in pup_ro1 with a 100 MB file (= 100 MB of I/O), instead of modifying the existing file (writing just the 4K delta to the right offset).

That's why writing duration comes to play. You can extend SSD life not only just limiting the write cycle but also limiting the writing cycle frequency

@dimkr
Copy link
Contributor Author

dimkr commented Aug 13, 2022

Closing this one, I don't care about aufs anymore. This behavior is risky when using overlay, and dc35a99 fixes this.

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

No branches or pull requests

5 participants