Page MenuHomePhabricator

Prevent worker queue leases from exceeding 64 characters
ClosedPublic

Authored by epriestley on Dec 16 2014, 11:14 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Mar 26, 11:42 PM
Unknown Object (File)
Sat, Mar 23, 10:29 PM
Unknown Object (File)
Sat, Mar 23, 1:25 PM
Unknown Object (File)
Sun, Mar 10, 4:06 PM
Unknown Object (File)
Mon, Mar 4, 5:39 PM
Unknown Object (File)
Mon, Mar 4, 4:53 PM
Unknown Object (File)
Mon, Mar 4, 1:56 PM
Unknown Object (File)
Feb 5 2024, 7:41 PM
Subscribers

Details

Summary

Ref T6742. Root cause of the issue:

  • Daemon was running on a machine with a very long host name, which produced a lease name which was longer than 64 characters.
  • MySQL wasn't set in STRICT_ALL_TABLES.
  • The daemon would UPDATE .. SET leaseOwner = <very long string> to lock a task, and MySQL would silently truncate.
  • The daemon would then try to select the locked task, but fail, because there's no matching lease owner.

To resolve this, use only the first 32 characters of the hostname. See IRC for more discussion.

Test Plan

Will confirm with reporter.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to Prevent worker queue leases from exceeding 64 characters.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
btrahan edited edge metadata.
btrahan added inline comments.
src/infrastructure/daemon/workers/query/PhabricatorWorkerLeaseQuery.php
259

should we use PhutilStringTruncator here instead? I am not sure how fancy these characters could end up being.

This revision is now accepted and ready to land.Dec 17 2014, 5:11 PM

I think we're safe assuming hostnames are ASCII, although I'm not 100% sure. I'll swap to StringTruncator just in case. T6768 should eventually provide a cleaner fix for this.

epriestley edited edge metadata.
  • Use PhutilUTF8StringTruncator.
  • Ran phd debug taskmaster with a very short truncation length and got sensible leaseOwner name.
This revision was automatically updated to reflect the committed changes.