Page MenuHomePhabricator
Differential D10998

Prevent worker queue leases from exceeding 64 characters
ClosedPublic
Actions

Authored by epriestley on Dec 16 2014, 11:14 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Dec 15, 12:27 PM
Unknown Object (File)
Sat, Nov 30, 7:23 AM
Unknown Object (File)
Fri, Nov 22, 11:08 PM
Unknown Object (File)
Wed, Nov 20, 12:59 AM
Unknown Object (File)
Oct 25 2024, 9:52 PM
Unknown Object (File)
Oct 14 2024, 8:35 AM
Unknown Object (File)
Oct 12 2024, 11:43 AM
Unknown Object (File)
Oct 6 2024, 6:28 AM
View All Files
Subscribers
epriestley

Details

Reviewers
btrahan
Maniphest Tasks
T6742: Daemon tasks not processing, repository reports "Fully Imported"
Commits
Restricted Diffusion Commit
rP7d9bda59a6bf: Prevent worker queue leases from exceeding 64 characters
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
Branch
longlease
Lint
Lint Passed
SeverityLocationCodeMessage
Advicesrc/infrastructure/daemon/workers/query/PhabricatorWorkerLeaseQuery.php:243XHP16TODO Comment
Unit
Tests Passed
Build Status
Buildable 3269
Build 3275: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

epriestley updated this revision to Diff 26410.Dec 16 2014, 11:14 PM
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.
epriestley added a task: T6742: Daemon tasks not processing, repository reports "Fully Imported".
Herald added a subscriber: epriestley. · View Herald TranscriptDec 16 2014, 11:14 PM
beeerd mentioned this in T6742: Daemon tasks not processing, repository reports "Fully Imported".Dec 16 2014, 11:15 PM
epriestley mentioned this in T6768: Worker queue lease names are unwieldy and could be better implemented.Dec 16 2014, 11:21 PM
btrahan accepted this revision.Dec 17 2014, 5:11 PM
btrahan edited edge metadata.
btrahan added inline comments.
src/infrastructure/daemon/workers/query/PhabricatorWorkerLeaseQuery.php
253

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
epriestley added a comment.Dec 17 2014, 5:19 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 updated this revision to Diff 26420.Dec 17 2014, 7:09 PM
epriestley edited edge metadata.
  • Use PhutilUTF8StringTruncator.
  • Ran phd debug taskmaster with a very short truncation length and got sensible leaseOwner name.
Closed by commit rP7d9bda59a6bf: Prevent worker queue leases from exceeding 64 characters (authored by epriestley, committed by epriestley). · Explain WhyDec 17 2014, 7:10 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
src/
infrastructure/
daemon/
workers/
query/
9 lines

Diff 26410

View Options

src/infrastructure/daemon/workers/query/PhabricatorWorkerLeaseQuery.php

Loading...
Phabricator · Built by Phacility