Skip to content

Conversation

@ankor2023
Copy link
Contributor

@ankor2023 ankor2023 commented Mar 13, 2025

Add a PID file name hash to the names of the shared memory segments and
Unix Domain Sockets. Since all instances running on the same host are
supposed to have unique PID files, this addition significantly reduces
the probability of name clashes when running multiple Squid instances
with the same service name (i.e. the same squid -n parameter value
that defaults to "squid").

A clash may still happen if two different PID file names have the same
hash or if multiple instances disable PID file management with
pid_filename none. Clashes may also happen in environments where Squid
does not even use service name for naming shared memory segments.

Examples of UDS and shared memory segment names (while using default
service name):

/var/run/squid/squid-SLWQ-kid-1.ipc
/var/run/squid/squid-SLWQ-coordinator.ipc
/dev/shm/squid-SLWQ-tls_session_cache.shm
/dev/shm/squid-SLWQ-transients_map_slices.shm

This change is a reference point for automated CONTRIBUTORS updates.

Add GetPidFilenameHash() prototype
Add Instance::GetPidFilenameHash() and murmur3_32() functions.
Prevent shared memory segment names clashes.
Updated comment
Add Instance uniq label to filename
Add PID filename hash label to the coordinator filename.
@squid-anubis squid-anubis added the M-failed-description https://github.com/measurement-factory/anubis#pull-request-labels label Mar 13, 2025
Code formatting
Code formatting
Code formatting
@squid-anubis squid-anubis removed the M-failed-description https://github.com/measurement-factory/anubis#pull-request-labels label Mar 13, 2025
@yadij
Copy link
Contributor

yadij commented Mar 13, 2025

Build issues are because the src/Instance.cc is not linked against the unit tests for Rock and UFS cache types. See src/Makefile.am "tests for fs/*". Please run "make check" locally to check the fix for these and find any other tests impacted by this new code.

Also, why the murmur hash? The original proposal was just to add the PID number (from the "master" or "coordinator" process). If we do need to hash for any reason Squid uses libnettle, for a selection of speed and security options.

@rousskov rousskov self-requested a review March 13, 2025 13:14
@rousskov rousskov added the S-waiting-for-author author action is expected (and usually required) label Mar 13, 2025
Copy link
Contributor

@rousskov rousskov left a comment

Choose a reason for hiding this comment

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

Amos: The original proposal was just to add the PID number

It was not. Proposal A was to add a hash of the full PID file name.

Adding a PID number instead of a hash would solve the problem with segment clashes as well but would create a worse problem: Segments left by crashed Squid instances will accumulate instead of being removed (because each instance is likely to have a unique master PID value, even instances that have the same PID file name).

Amos: If we do need to hash for any reason Squid uses libnettle, for a selection of speed and security options.

I agree that this PR should not introduce a new hash function. However, please do not rely on libnettle for this PR either; essentially, ignore that libnettle exists. See one of the change requests for a specific hash function recommendation.

@ankor2023

This comment was marked as outdated.

Replace mumur for MD5
@rousskov rousskov removed the request for review from kinkie July 8, 2025 15:43
@rousskov rousskov removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box M-failed-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels labels Jul 8, 2025
@rousskov rousskov added M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels and removed backport-to-v7 maintainer has approved these changes for v7 backporting labels Jul 8, 2025
squid-anubis pushed a commit that referenced this pull request Jul 8, 2025
Add a PID file name hash to the names of the shared memory segments and
Unix Domain Sockets. Since all instances running on the same host are
supposed to have unique PID files, this addition significantly reduces
the probability of name clashes when running multiple Squid instances
with the same service name (i.e. the same `squid -n` parameter value
that defaults to "squid").

A clash may still happen if two different PID file names have the same
hash or if multiple instances disable PID file management with
`pid_filename none`. Clashes may also happen in environments where Squid
does not even use service name for naming shared memory segments.

Examples of UDS and shared memory segment names (while using default
service name):

    /var/run/squid/squid-SLWQ-kid-1.ipc
    /var/run/squid/squid-SLWQ-coordinator.ipc
    /dev/shm/squid-SLWQ-tls_session_cache.shm
    /dev/shm/squid-SLWQ-transients_map_slices.shm

This change is a reference point for automated CONTRIBUTORS updates.
@squid-anubis squid-anubis added the M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels label Jul 8, 2025
@rousskov
Copy link
Contributor

rousskov commented Jul 8, 2025

@kinkie, I have removed backport-to-v7 label just in case you do not want to backport this PR that might trigger more PSHMNAMLEN errors in some cases (or want to wait for a followup PR as discussed earlier). Please restore that label if needed!

@squid-anubis squid-anubis added M-failed-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels and removed M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels labels Jul 8, 2025
@rousskov
Copy link
Contributor

rousskov commented Jul 8, 2025

Newer FreeBSD version for package ztrack:
To ignore this error set IGNORE_OSVERSION=yes
  - package: 1305000
  - running userland: 1304000
repository FreeBSD contains packages for wrong OS version

@kinkie, if restarting that failed FreeBSD v13.4 staging test is not the answer, then please help getting past that package versioning error.

Edit: @yadij has already flagged the same problem at #2110 (comment). I will just follow the discussion in that PR to reduce the noise here.

@kinkie
Copy link
Contributor

kinkie commented Jul 8, 2025

CI unblocked by PR #2112

@kinkie kinkie added S-waiting-for-PR Closure of other PR(s), current or future, is expected (and usually required) backport-to-v7 maintainer has approved these changes for v7 backporting backport-to-v6 and removed M-failed-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels S-waiting-for-PR Closure of other PR(s), current or future, is expected (and usually required) labels Jul 8, 2025
squid-anubis pushed a commit that referenced this pull request Jul 9, 2025
Add a PID file name hash to the names of the shared memory segments and
Unix Domain Sockets. Since all instances running on the same host are
supposed to have unique PID files, this addition significantly reduces
the probability of name clashes when running multiple Squid instances
with the same service name (i.e. the same `squid -n` parameter value
that defaults to "squid").

A clash may still happen if two different PID file names have the same
hash or if multiple instances disable PID file management with
`pid_filename none`. Clashes may also happen in environments where Squid
does not even use service name for naming shared memory segments.

Examples of UDS and shared memory segment names (while using default
service name):

    /var/run/squid/squid-SLWQ-kid-1.ipc
    /var/run/squid/squid-SLWQ-coordinator.ipc
    /dev/shm/squid-SLWQ-tls_session_cache.shm
    /dev/shm/squid-SLWQ-transients_map_slices.shm

This change is a reference point for automated CONTRIBUTORS updates.
@squid-anubis squid-anubis added the M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels label Jul 9, 2025
@squid-anubis squid-anubis added M-merged https://github.com/measurement-factory/anubis#pull-request-labels and removed M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels labels Jul 9, 2025
@rousskov
Copy link
Contributor

rousskov commented Jul 10, 2025

  • .... PR that automatically replaces long segment names with short hashes
  • .... PR that manually renames existing segments

To add one more data point for the future followup PR(s) to consider, here is a segment name I just saw in the logs produced by an older (v5- or v6-based?) Squid:

kid3| 54,3| mem/Segment.cc(148) open: opened /squid-usr.local.squid.var.cache.rocks.1_map_filenos.shm segment: 260

That name is 55-characters long and cannot be shortened enough by simply shortening map_filenos or other hard-coded labels because its longest component is derived from the admin-configured 33-character cache_dir location: /usr/local/squid/var/cache/rocks/1. This is why I crossed out the "manually renames existing segments" PR idea above.

If we want to fully support SMP Squid on MacOS, then we need a different way to produce short names: Either a hash or some kind of enumeration. To avoid completely mishandling, for example, instances where different kids have a different set of cache_dir directives or a different cache_dir directives order, both options would have to use a central hashing/enumeration authority (e.g., a new special/dedicated shared memory segment mapping full names to hashes or enumeration IDs).

kinkie pushed a commit to kinkie/squid that referenced this pull request Sep 13, 2025
…cache#2023)

Add a PID file name hash to the names of the shared memory segments and
Unix Domain Sockets. Since all instances running on the same host are
supposed to have unique PID files, this addition significantly reduces
the probability of name clashes when running multiple Squid instances
with the same service name (i.e. the same `squid -n` parameter value
that defaults to "squid").

A clash may still happen if two different PID file names have the same
hash or if multiple instances disable PID file management with
`pid_filename none`. Clashes may also happen in environments where Squid
does not even use service name for naming shared memory segments.

Examples of UDS and shared memory segment names (while using default
service name):

    /var/run/squid/squid-SLWQ-kid-1.ipc
    /var/run/squid/squid-SLWQ-coordinator.ipc
    /dev/shm/squid-SLWQ-tls_session_cache.shm
    /dev/shm/squid-SLWQ-transients_map_slices.shm

This change is a reference point for automated CONTRIBUTORS updates.
squidadm pushed a commit to squidadm/squid that referenced this pull request Oct 2, 2025
…cache#2023)

Add a PID file name hash to the names of the shared memory segments and
Unix Domain Sockets. Since all instances running on the same host are
supposed to have unique PID files, this addition significantly reduces
the probability of name clashes when running multiple Squid instances
with the same service name (i.e. the same `squid -n` parameter value
that defaults to "squid").

A clash may still happen if two different PID file names have the same
hash or if multiple instances disable PID file management with
`pid_filename none`. Clashes may also happen in environments where Squid
does not even use service name for naming shared memory segments.

Examples of UDS and shared memory segment names (while using default
service name):

    /var/run/squid/squid-SLWQ-kid-1.ipc
    /var/run/squid/squid-SLWQ-coordinator.ipc
    /dev/shm/squid-SLWQ-tls_session_cache.shm
    /dev/shm/squid-SLWQ-transients_map_slices.shm

This change is a reference point for automated CONTRIBUTORS updates.
yadij pushed a commit that referenced this pull request Oct 2, 2025
Add a PID file name hash to the names of the shared memory segments and
Unix Domain Sockets. Since all instances running on the same host are
supposed to have unique PID files, this addition significantly reduces
the probability of name clashes when running multiple Squid instances
with the same service name (i.e. the same `squid -n` parameter value
that defaults to "squid").

A clash may still happen if two different PID file names have the same
hash or if multiple instances disable PID file management with
`pid_filename none`. Clashes may also happen in environments where Squid
does not even use service name for naming shared memory segments.

Examples of UDS and shared memory segment names (while using default
service name):

    /var/run/squid/squid-SLWQ-kid-1.ipc
    /var/run/squid/squid-SLWQ-coordinator.ipc
    /dev/shm/squid-SLWQ-tls_session_cache.shm
    /dev/shm/squid-SLWQ-transients_map_slices.shm

This change is a reference point for automated CONTRIBUTORS updates.
@yadij yadij removed the backport-to-v7 maintainer has approved these changes for v7 backporting label Oct 3, 2025
@yadij
Copy link
Contributor

yadij commented Oct 3, 2025

backport to v7 completed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

M-merged https://github.com/measurement-factory/anubis#pull-request-labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants