Skip to content

Update MerlinAU.sh Logging Solution to Stop Duplicating Mount Points#48

Merged
Martinski4GitHub merged 4 commits intodevfrom
ExtremeFiretop-Logging-Patch
Dec 23, 2023
Merged

Update MerlinAU.sh Logging Solution to Stop Duplicating Mount Points#48
Martinski4GitHub merged 4 commits intodevfrom
ExtremeFiretop-Logging-Patch

Conversation

@ExtremeFiretop
Copy link
Owner

Tested and successfully solves my issues as discussed here: #47

@ExtremeFiretop
Copy link
Owner Author

ExtremeFiretop commented Dec 22, 2023

I also adjusted the timeouts for the actual flash/curl.

I know, I hear you saying "WHAT!?!?!?! WE JUST DISCUSSED THIS?!"

But, hear me out, I am actually doing flashes with the curl over and over again in all these tests while I know you have only done it once so far. The reason I'm extending the actual reboot timer, and lowering the curl timer, why?

Because 99% of the time, the curl completes instantly, and the script moves on, that's not the correct location where we should put longer wait/pause.. The upload is done instantly over it's own "local interface". There is no actual "networking" being done on the "upload" since it's already local. Normally we wait for the upload to complete from our device to the router, but in this case it returns within seconds because it's local already.

The kill wait we implemented is literally only for that 1% of the time when it fails.
it will timeout and kill at 3 and a half minutes if the curl doesn't instantly return like it does 99% of the time. (Remember, it's instant because we aren't "Uploading" anything to local storage)

The rest of the time (99%) we want the longest wait to be once the curl completes and turns, it "should" reboot itself once updated, but if it doesn't after 90 seconds we handle it for the router after about 90 seconds once the "curl/upload" is done.

Basically:

  1. Upload Firmware Request (Curl)

  2. Wait for Upload to complete...
    (Used to be 4 minutes, but now 3 and a half) Normally Instant due to being local already!

  3. Once Curl is completed. Start Flash

  4. Wait for Flash to complete
    (Used to be 30 seconds.... Woof we were probably cutting it close... Now extended to 1 minute and 30 seconds)

  5. Reboot

@Martinski4GitHub Martinski4GitHub merged commit 79ee149 into dev Dec 23, 2023
@Martinski4GitHub
Copy link
Collaborator

Martinski4GitHub commented Dec 23, 2023

I also adjusted the timeouts for the actual flash/curl.

I know, I hear you saying "WHAT!?!?!?! WE JUST DISCUSSED THIS?!"

But, hear me out, I am actually doing flashes with the curl over and over again in all these tests while I know you have only done it once so far. The reason I'm extending the actual reboot timer, and lowering the curl timer, why?

Ah, OK. I get it now. That makes sense and explains why with the previous "sleep 60" the F/W Update would work most of the time since, as you said, the "curl" command itself likely takes less than a second to complete; the rest is the actual flashing of the image after shutting down most services, any external user space processes (e.g. Entware services, scripts, etc.) and unmounting any USB-attached drives. So I agree that 90 seconds should be sufficient. Good call.

@ExtremeFiretop
Copy link
Owner Author

I also adjusted the timeouts for the actual flash/curl.
I know, I hear you saying "WHAT!?!?!?! WE JUST DISCUSSED THIS?!"
But, hear me out, I am actually doing flashes with the curl over and over again in all these tests while I know you have only done it once so far. The reason I'm extending the actual reboot timer, and lowering the curl timer, why?

Ah, OK. I get it now. That makes sense and explains why with the previous "sleep 60" the F/W Update would work most of the time since, as you said, the "curl" command itself likely takes less than a second to complete; the rest is the actual flashing of the image after shutting down most services, any external user space processes (e.g. Entware services, scripts, etc.) and unmounting any USB-attached drives. So I agree that 90 seconds should be sufficient. Good call.

Bingo! I hope that 90 seconds is long enough. Really what we could do is extend the 90 to 3 minutes like the default. So we 'allow' the curl to take up to 3, but expect it to be done within seconds, and then 'allow" the flash and reboot to take up to 3 minutes, or we reboot it ourselves.

I think that might even be better before we promote to production, but I'll wait on your opinions, your 2 cents are always appreciated haha.

@Martinski4GitHub
Copy link
Collaborator

Bingo! I hope that 90 seconds is long enough. Really what we could do is extend the 90 to 3 minutes like the default. So we 'allow' the curl to take up to 3, but expect it to be done within seconds, and then 'allow" the flash and reboot to take up to 3 minutes, or we reboot it ourselves.

I was thinking about that as well while making the changes for PR #49. I fully agree that to be on the conservative side we should increase the 90-sec wait before rebooting to 3 minutes. IOW, both wait times (one for the curl command & one for reboot) should be 3 minutes. My preference is to err on the side of caution.

@ExtremeFiretop
Copy link
Owner Author

Bingo! I hope that 90 seconds is long enough. Really what we could do is extend the 90 to 3 minutes like the default. So we 'allow' the curl to take up to 3, but expect it to be done within seconds, and then 'allow" the flash and reboot to take up to 3 minutes, or we reboot it ourselves.

I was thinking about that as well while making the changes for PR #49. I fully agree that to be on the conservative side we should increase the 90-sec wait before rebooting to 3 minutes. IOW, both wait times (one for the curl command & one for reboot) should be 3 minutes. My preference is to err on the side of caution.

Do it.

I think we originally had something like this (6 minutes total) but we compromised, but we compromised the wrong way hahaha! It's okay we caught it early and live and learn. 3 and 3 is the safest bet.

@Martinski4GitHub
Copy link
Collaborator

Bingo! I hope that 90 seconds is long enough. Really what we could do is extend the 90 to 3 minutes like the default. So we 'allow' the curl to take up to 3, but expect it to be done within seconds, and then 'allow" the flash and reboot to take up to 3 minutes, or we reboot it ourselves.

I was thinking about that as well while making the changes for PR #49. I fully agree that to be on the conservative side we should increase the 90-sec wait before rebooting to 3 minutes. IOW, both wait times (one for the curl command & one for reboot) should be 3 minutes. My preference is to err on the side of caution.

Do it.

I think we originally had something like this (6 minutes total) but we compromised, but we compromised the wrong way hahaha! It's okay we caught it early and live and learn. 3 and 3 is the safest bet.

Indeed we did. We had the right idea but the wrong locations. I'll make the changes now.

@ExtremeFiretop ExtremeFiretop deleted the ExtremeFiretop-Logging-Patch branch December 23, 2023 05:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants