search

mynodebtc / mynode

The easiest way to run Bitcoin and Lightning!
Other
649 stars 146 forks source link

log2ram for x86? #868

Open cwiggs opened 3 months ago

cwiggs commented 3 months ago

Describe the bug This might be a question more than anything. I noticed in the setup_device.sh script here: https://github.com/mynodebtc/mynode/blob/master/setup/setup_device.sh#L457 it installs log2ram if the system is a RPI or x86 device.

When trying to run this on a Debian 12 VM in proxmox log2ram failed to start. I ended up just disabling it all together since I don't really want logs going to ram on this system. I was thinking it might be better to check if the system is an sdcard and if so setup log2ram.

I'd be willing to submit a PR if you think that is a good idea.

Expected behavior log2ram should only be enabled if the system is using an sd card.

Screenshots n/a

Desktop (please complete the following information): n/a

MyNode hardware (please complete the following information):

Additional context

tehelsper commented 3 months ago

Yeah, that change would be beneficial. The model two (which is x86) boots off of a SD card or USB flash drive, so this was added as an optimization for that platform.

It may be better to leave it installed and just disable log2ram if it's not on an SD card during startup or add an option to enable/disable on the settings page manually. The images are typically made using log2ram while they are on an USB drive, so any detection done during setup_device.sh may not represent the final drive the device is actually using. There's also one other install location in mynode_post_upgrade.sh.

cwiggs commented 3 months ago

The model two (which is x86) boots off of a SD card or USB flash drive, so this was added as an optimization for that platform.

Ah okay that makes sense.

It may be better to leave it installed and just disable log2ram if it's not on an SD card during startup or add an option to enable/disable on the settings page manually.

Yeah that makes sense. For the time being I'll do some work to see why it's failing on Debian 12 and either fix it or create an option to disable starting the service.

The images are typically made using log2ram while they are on an USB drive, so any detection done during setup_device.sh may not represent the final drive the device is actually using.

Okay. I'm just using the setup_device.sh script on a Debian 12 VM so I'm not sure how that merges with the building the image logic. Any suggestions and how to deal with this?

There's also one other install location in mynode_post_upgrade.sh.

I see there is a lot of duplicated code in mynode_post_upgrade.sh and setup_device.sh, are you okay if I create a libs directory and put some simple bash scripts in there with common functions that we can import (source) in both of the scripts? This would allow us to write the code once and just use it when we need it in multiple places.

cwiggs commented 3 months ago

For the time being I'll do some work to see why it's failing on Debian 12 and either fix it or create an option to disable starting the service.

I was able to fix the issue with Debian 12, PR is her: https://github.com/mynodebtc/mynode/pull/869

I still think it's good to do a few things:

Let me know what you think about the above things, and the questions from my previous comment and I'd be happy to support PRs for all of this.

Thanks for the app, MyNode has been super helpful for me, I just switched to it from Umbrel.

tehelsper commented 3 months ago

Glad to hear you've been liking it!

Okay. I'm just using the setup_device.sh script on a Debian 12 VM so I'm not sure how that merges with the building the image logic. Any suggestions and how to deal with this?

OK, I see how you've been using setup_device. That's basically the use case, when running it on a base debian distro, it will add and install and the default apps and software. Your PR change was good.

I see there is a lot of duplicated code in mynode_post_upgrade.sh and setup_device.sh, are you okay if I create a libs directory and put some simple bash scripts in there with common functions that we can import (source) in both of the scripts? This would allow us to write the code once and just use it when we need it in multiple places.

I see there is a lot of duplicated code in mynode_post_upgrade.sh and setup_device.sh

There is a lot of duplication between the two, but they have different use cases and at this point, that code almost never needs to change. I'd prefer not to change it up at this point. A lot duplicate code that does change was pulled out into a separate script and all future apps use the SDK approach so they go into their own locations without needing to be in either setup_device.sh or mynode_post_upgrade.sh. You can see some of that in mynode_app_versions.sh and rootfs/standard/usr/share/mynode_apps/.

I like the second suggested option the most. The first could be nice, but I worry it could be error-prone. I'm not sure how to consistently detect the type of hardware.