mstormi / openhabian-zram

zram extensions for openHABian
MIT License
3 stars 0 forks source link

Rewrite zram-config #5

Closed ecdye closed 4 years ago

ecdye commented 4 years ago

Signed-off-by: Ethan Dye mrtops03@gmail.com

ecdye commented 4 years ago

@mstormi I think this has it working with the zramsync service. Try it on your end and see if it works for you too. I have it working on my end so it should work for you (fingers crossed).

ecdye commented 4 years ago

I'll make the necessary changes to install it on the openhabian zramtest branch. Make to delete /opt/zram before reinstalling because of the changed repo location.

mstormi commented 4 years ago

I had temporarily implemented another service for stop-only, too, but I want to merge changes back to the original repo and the original author is not happy with adding another service so I tried without and it worked for me. My understanding was that RequiresMountsFor= will force startup to wait for mounts, not to stop if they're unavailable. Then again, giving it another thought now, waiting for ZRAM upper FS mounts in ZRAM startup service obviously cannot work, so we need another service (and I have no idea why it worked without for me).

But getting back to the idea of merging changes back into the original: do we need your changes to zram-config or is that just the "usual" lintering ? Why removal of root-ro.sh ?

mstormi commented 4 years ago

Installed from zramtest branch but both services were down after start (zramsync did start+stop, zram-config only started according to journalctl -xu)

One reason (others may exist):

-- The job identifier is 1131.
Aug 25 15:07:35 openhab systemd[1]: zram-config.service: Found ordering cycle on local-fs.target/stop
Aug 25 15:07:35 openhab systemd[1]: zram-config.service: Found dependency on srv-openhab2\x2dlogs.mount/stop
Aug 25 15:07:35 openhab systemd[1]: zram-config.service: Found dependency on zram-config.service/stop
Aug 25 15:07:35 openhab systemd[1]: zram-config.service: Job local-fs.target/stop deleted to break ordering cycle starting with zram-config.service/stop
Aug 25 15:07:39 openhab systemd[1]: zram-config.service: Succeeded.
ecdye commented 4 years ago

Try uninstalling and then reinstalling using the menu, because of script changes I had to run it a few times to get it to properly uninstall.

Also try rebooting after uninstalling to make sure any remnants are removed (specifically any zram dirs that failed to unmount). I also deleted the /opt/zram and /storage/zram folders to make sure that no conflicting changes were present.

ecdye commented 4 years ago

I had temporarily implemented another service for stop-only, too, but I want to merge changes back to the original repo and the original author is not happy with adding another service so I tried without and it worked for me.

I don't think that we can merge them back if we want to play by his rules. I don't know why it worked for you either, but this is working for me flawlessly.

My understanding was that RequiresMountsFor= will force startup to wait for mounts, not to stop if they're unavailable.

That is not what was happening in my observation it seemed to never start on reboot.

But getting back to the idea of merging changes back into the original: do we need your changes to zram-config or is that just the "usual" lintering ? Why removal of root-ro.sh?

Not necessarily, they are lintering changes and also logging changes, overall they make the code much simpler and more readable. I removed the ephemeral mode because it has no real use in our openhabian project as we don't support that.

I also removed the PartOf=openhab2.service and After=openhab2.service smbd.service because in my testing at times those caused zram to not start on boot but when I removed them it always started. Is there any real issue with removing those? If so then we should look into another workaround as it is unacceptable that zram should not start at boot.

mstormi commented 4 years ago

Not necessarily, they are lintering changes and also logging changes, overall they make the code much simpler and more readable. I removed the ephemeral mode because it has no real use in our openhabian project as we don't support that.

I would like not to enforce this sort of branching. Can you re-include ephemeral mode and propose your other changes to zram-config as a PR to the original repo ? We should also propose the zramsync changes once we have sorted them out, but as a separate PR so if the author dislikes that he can still merge your zram-config enhancements. wdyt ?

I also removed the PartOf=openhab2.service and After=openhab2.service smbd.service because in my testing at times those caused zram to not start on boot but when I removed them it always started. Is there any real issue with removing those?

Hmm. After=openhab2.service seems wrong, do you mean Before= ? PartOf= stops ZRAM when OH is stopped. This was meant to sync ZRAM but if sync works on reboot that should be sufficient.

/srv, hmm. It used to be like this: zram must start before srv-openhab.mount and those before smbd. srv-openhab does loopback mounts of some OH dirs to /srv/openhab, one of them (userdata) is /var/lib/openhab2 which was on ZRAM earlier so ZRAM had to be running by the time those srv- start. smbd must be last as it shares /srv/*. Now I changed ZRAM is no longer all of var/lib/openhab2 but just 2 subdirs of that.

Frankly I'm unsure what happens if we loopback-mount and Samba-share /srv/openhab2-userdata before we start ZRAM, then access /srv/openhab2-userdata/persistence. Will we get to the the upper FS or the lower FS from before ? That once was a problem and the reason to remove fstab and introduce openhab2-*.mount Can you test ?

ecdye commented 4 years ago

Not necessarily, they are lintering changes and also logging changes, overall they make the code much simpler and more readable. I removed the ephemeral mode because it has no real use in our openhabian project as we don't support that.

I would like not to enforce this sort of branching. Can you re-include ephemeral mode and propose your other changes to zram-config as a PR to the original repo ? We should also propose the zramsync changes once we have sorted them out, but as a separate PR so if the author dislikes that he can still merge your zram-config enhancements. wdyt ?

I can make them, I don't know that they will be accepted.

I also removed the PartOf=openhab2.service and After=openhab2.service smbd.service because in my testing at times those caused zram to not start on boot but when I removed them it always started. Is there any real issue with removing those?

Hmm. After=openhab2.service seems wrong, do you mean Before= ?

Yes.

PartOf= stops ZRAM when OH is stopped. This was meant to sync ZRAM but if sync works on reboot that should be sufficient.

Then we should be fine there.

/srv, hmm. It used to be like this: zram must start before srv-openhab.mount and those before smbd. srv-openhab does loopback mounts of some OH dirs to /srv/openhab, one of them (userdata) is /var/lib/openhab2 which was on ZRAM earlier so ZRAM had to be running by the time those srv- start. smbd must be last as it shares /srv/*. Now I changed ZRAM is no longer all of var/lib/openhab2 but just 2 subdirs of that.

Frankly I'm unsure what happens if we loopback-mount and Samba-share /srv/openhab2-userdata before we start ZRAM, then access /srv/openhab2-userdata/persistence. Will we get to the the upper FS or the lower FS from before ? That once was a problem and the reason to remove fstab and introduce openhab2-*.mount Can you test ?

I don't fully understand what you are saying. Shouldn't ZRAM always start first because it is linked to basic.target which should always go first.

I would like to get this in ASAP that way we stop having data loss. I will readd ephemeral and then we should be ready, then I will work on merging them back up to the original repo.

mstormi commented 4 years ago

I can make them, I don't know that they will be accepted.

Yes please I was hoping for that. Maybe he doesn't know yet that we know each other :) BTW you can also win the whole project if you like. He offered to transfer it (see open issue).

PartOf= stops ZRAM when OH is stopped. Then we should be fine there.

There was another reason we need to cross check (talking to myself :-) so you can validate): Back when I ZRAM'ed all of /var/lib/openhab2, this included the OH cache and tmp dirs, easily filling available ZRAM space. It was Benjy to propose PartOf= ... by stopping ZRAM, too, on OH stop, this forced a ZRAM sync and on next start the just-created cache was on lower FS and no longer occupying that much space in upper in RAM. But I think now that we excluded that from ZRAM this is no longer a valid reason for PartOf.

I don't fully understand what you are saying. Shouldn't ZRAM always start first because it is linked to basic.target which should always go first.

So you mean because /srv mount have WantedBy=multi-user.target (which is after basic.target? not sure ....) that enforces /srv mount to be executed after ZRAM ? No I don't think so. Any service can start as soon as its dependencies are met. So if we don't put After=zram-config.service into /srv/... .mount, we cannot be sure they're done after ZRAM is up. That's the crux with this whole thing: if you don't enforce an explicit dependency it's coincidence which one gets started first. This is why at times current ZRAM implementation syncs and at times it does not.

ecdye commented 4 years ago

I can make them, I don't know that they will be accepted.

Yes please I was hoping for that. Maybe he doesn't know yet that we know each other :) BTW you can also win the whole project if you like. He offered to transfer it (see open issue).

Once we have it stable and working over here I'll work with you to create a perfect PR that will hopefully be accepted.

PartOf= stops ZRAM when OH is stopped. Then we should be fine there.

There was another reason we need to cross check (talking to myself :-) so you can validate): Back when I ZRAM'ed all of /var/lib/openhab2, this included the OH cache and tmp dirs, easily filling available ZRAM space. It was Benjy to propose PartOf= ... by stopping ZRAM, too, on OH stop, this forced a ZRAM sync and on next start the just-created cache was on lower FS and no longer occupying that much space in upper in RAM. But I think now that we excluded that from ZRAM this is no longer a valid reason for PartOf.

Ok, sounds good we should be fine for that then.

I don't fully understand what you are saying. Shouldn't ZRAM always start first because it is linked to basic.target which should always go first.

So you mean because /srv mount have WantedBy=multi-user.target (which is after basic.target? not sure ....) that enforces /srv mount to be executed after ZRAM ? No I don't think so. Any service can start as soon as its dependencies are met. So if we don't put After=zram-config.service into /srv/... .mount, we cannot be sure they're done after ZRAM is up. That's the crux with this whole thing: if you don't enforce an explicit dependency it's coincidence which one gets started first. This is why at times current ZRAM implementation syncs and at times it does not.

I can add smbd.service back and hopefully it still always starts on reboot.

mstormi commented 4 years ago

I can add smbd.service back and hopefully it still always starts on reboot.

It needs to work with Before=openhab2.service, too, doesn't it ? ZRAM needs to be up before OH starts. If that dependency isn't fulfilled we'll have bad (outdated) persistence data so we should use systemd to enforce proper order. When it doesn't start on boot check for a dependency loop in journalctl.

Maybe there is a problem because you linked zramsync to zram-config ? BTW, is that linking required, I don't think so ? zram-config is on boot, zramsync is on shutdown

ecdye commented 4 years ago

I can add smbd.service back and hopefully it still always starts on reboot.

It needs to work with Before=openhab2.service, too, doesn't it ? ZRAM needs to be up before OH starts. If that dependency isn't fulfilled we'll have bad (outdated) persistence data so we should use systemd to enforce proper order. When it doesn't start on boot check for a dependency loop in journalctl.

I'll look into it, and then readd it.

Maybe there is a problem because you linked zramsync to zram-config ? BTW, is that linking required, I don't think so ? zram-config is on boot, zramsync is on shutdown

It is, it is so that the stop service only ever runs if zram-config was started properly.

mstormi commented 4 years ago

Unfortunately while this installed fine, zram was unavaible after first reboot.

-- Reboot --
Aug 27 23:20:35 openhab systemd[1]: local-fs.target: Found ordering cycle on srv-openhab2\x2duserdata.mount/start
Aug 27 23:20:35 openhab systemd[1]: local-fs.target: Found dependency on zram-config.service/start
Aug 27 23:20:35 openhab systemd[1]: local-fs.target: Found dependency on basic.target/start
Aug 27 23:20:35 openhab systemd[1]: local-fs.target: Found dependency on sockets.target/start
Aug 27 23:20:35 openhab systemd[1]: local-fs.target: Found dependency on dbus.socket/start
Aug 27 23:20:35 openhab systemd[1]: local-fs.target: Found dependency on sysinit.target/start
mstormi commented 4 years ago

Hmm. I fixed an error in the timesyncd dhcpcd hook. Ever since, ZRAM works after boot. I'm still wary though because I cannot explain the dependency so it may be coincidence as well and I've just not tested enough.