Skip to content

Adjust imgsrv metrics#200

Merged
aelkiss merged 2 commits intomainfrom
ETT-1448-imgsrv-metrics
Apr 15, 2026
Merged

Adjust imgsrv metrics#200
aelkiss merged 2 commits intomainfrom
ETT-1448-imgsrv-metrics

Conversation

@aelkiss
Copy link
Copy Markdown
Member

@aelkiss aelkiss commented Apr 13, 2026

Partially reverts #13

  • Use /ram/imgsrv-metrics for a consistent location for shared location for updating metrics (/tmp in Debian 13 is too transient and has caused issues)

  • Remove Utils::MonitorRun -- this was pretty hacky and we haven't ended up using the metrics from it.

If this continues to cause issues, we may want to back off on metrics here entirely and instead analyze metrics from the logs until we have a better sense of what to record & a stable way to record it.

* Use /ram/imgsrv-metrics for a consistent location for shared location
  for updating metrics (/tmp in Debian 13 is too transient and
  has caused issues)

* Remove Utils::MonitorRun -- this was pretty hacky and we haven't ended
  up using the metrics from it.

If this continues to cause issues, we may want to back off on metrics
here entirely and instead analyze metrics from the logs until we have a
better sense of what to record & a stable way to record it.
@aelkiss aelkiss requested a review from moseshll April 13, 2026 18:57
@aelkiss
Copy link
Copy Markdown
Member Author

aelkiss commented Apr 13, 2026

I'll want to verify this works as expected in dev before merging it. I'm not sure what the interaction will be between the persistent PSGI imgsrv and other things that might try to update the stats, and I don't remember how exactly this was working before.

If we can't get it working reliably, we can just turn it off.

@aelkiss
Copy link
Copy Markdown
Member Author

aelkiss commented Apr 13, 2026

Probably not super urgent as @rrotter has put another fix in place to avoid the Debian 13 behavior of trying to automatically clean out old stuff in /tmp

Comment thread mdp-lib/Utils/Extract.pm

my @yes;
# Pipe echo to unzip so it won't hang on a user prompt when ramdisk is full
push @yes, "echo", "n";
Copy link
Copy Markdown
Contributor

@moseshll moseshll Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is "yes" saying "no" to unzip? I haven't ventured much into this corner of imgsrv, so it raises the question for me, what question is unzip prompting us to answer? And what is the resulting behavior when we say "n" to whatever question this is? (I realize the handling of the unzip call is peripheral to what we're really doing here, but since we're here...)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good question...

Copy link
Copy Markdown
Contributor

@moseshll moseshll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment-in-passing. Nothing here fills me with dread. APPROVE

@rrotter
Copy link
Copy Markdown
Contributor

rrotter commented Apr 14, 2026

Continuing to use /ram is a bit redundant, since /tmp is also tmpfs:

# df -h /ram /tmp
Filesystem      Size  Used Avail Use% Mounted on
tmpfs           4.0G  1.4G  2.7G  35% /ram
tmpfs            18G  2.0M   18G   1% /tmp

I'd like /tmp to be usable, and not something we avoid because of systemd weirdness, and personally I'd work towards deprecating /ram. Up to you of course, just my $0.02.

@aelkiss
Copy link
Copy Markdown
Member Author

aelkiss commented Apr 14, 2026

Continuing to use /ram is a bit redundant, since /tmp is also tmpfs:

# df -h /ram /tmp
Filesystem      Size  Used Avail Use% Mounted on
tmpfs           4.0G  1.4G  2.7G  35% /ram
tmpfs            18G  2.0M   18G   1% /tmp

I'd like /tmp to be usable, and not something we avoid because of systemd weirdness, and personally I'd work towards deprecating /ram. Up to you of course, just my $0.02.

Since imgsrv is already using /ram quite extensively, having it all be in the same place for the time being makes sense to me, BUT in the future we could definitely try to get rid of /ram.

@aelkiss aelkiss merged commit 1c28e65 into main Apr 15, 2026
2 checks passed
@aelkiss aelkiss deleted the ETT-1448-imgsrv-metrics branch April 15, 2026 13:59
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

Successfully merging this pull request may close these issues.

3 participants