@epriestley: Jaime, Senior DBA at the WMF, has posted some answers for your list of questions:
- Queries
- All Stories
- Search
- Advanced Search
- Transactions
- Transaction Logs
Advanced Search
Oct 12 2016
@jessjohnson I still haven't had a chance to dig further into this, but I'll report back when I do (or if you figure anything out, please let me know). I don't think killing off the tasks permanently is really an option since this seems to happen for specific degenerate instances of otherwise-needed tasks. Perhaps not allowing these tasks to retry would be an okay bandaid, but I'd still like to figure out the underlying problem if we can.
Oct 11 2016
Also, like @jcox, the diffs always load in that advanced space-age script like there's no problem. Perhaps these exceptions are aware of our attempts to end them. @epriestley Do you have an idea of something else we could try to see why so many exceptions are occurring?
The upstream still doesn't have a reproduction case; we won't act until we have one. See Providing Reproduction Steps.
We're also seeing about 100k of these failures in our logs over the past few weeks. I clear out all the tasks and they quickly return. Have you found the root cause?
That change doesn't specifically do anything about mail tasks, but assuming they were coming out of repository imports it should probably clear them up too. I'd expect queue behavior to generally be more reasonable around repository imports once you pick up those changes.
Nope we don't have that integrated in yet (just missed our last cutoff). Once we get that pulled in I'll confirm it clears up the issue.
I tried adding a build-step to collect the summary from jenkins, but it turns out that the next step doesn't run anyway if the first step failed (Which is the case I'm more interested in), so back to square one with this.
Did you experience these issues after D16585? My expectation is that it should be almost impossible for PhabricatorRepositoryCommitHeraldWorker tasks to stall the queue for other priority-2000 tasks after that change.
Well, just Conduit for editing -- querying is already in D16592.
I think making authorizations transactional isn't too much work. They already have PHIDs, and aren't edited in very many places.
I'm comfortable bringing this stuff upstream today:
I'm not sure there's a way to do that's easier than cloning the repository. You can use git clone --depth 1 to only load the current version of each file, instead of the entire history.
Evan, do you think there is a route forward with this that would be feasible for me to implement upstream?
This will require us to leave login credentials on the server that is pulling the file, no? I was more interested in some limiting the scope of access to a single project -- thereby something like <some_access_key>.
Oct 10 2016
Yeah, my problem is explaining to users what they should do, no so much with the actual blocking configuration. Although, if they do arc-diff and the only problem is my landing script not working, than I can special case it on my end instead of teaching them things.
(That doesn't solve the "engineers don't know how to initialize a new Git repository with git commands" problem, but I think that one's a bit more involved.)
I'm assuming the "empty" issue is that you have a rule like this?
The problem I'm facing is that users just don't know how to push code to a new branch; They expect to git clone a repository that already "works"; Commands like git remote or git push with any arguments are simply beyond the git-fu of some engineers.
If the major immediate issue is Herald disallowing branch creation with git push, we could add a Herald condition like "Repository is empty".
Another hitch when creating a git repo, is that it has no branches, which is confusing for less-advanced users, and very confusing if you otherwise dissallow git push; GH solves this by adding a README file to create a branch with a commit.
I think a more powerful approach is to support some kind of "templates", so that we can create a new "Android App" repo or "COM Module" repo directly, similar to how IDEs have project templates.
Here's what I'm planning to do for imports:
Oct 9 2016
Also, reindexing is required after switching to innodb.
Does InnoDB FULLTEXT respect ft_boolean_syntax? If no: is there another similar option? If no: we need to parse queries ourselves and submit the +A +B version to the backend (see also T10642).
That's some pretty awesome support for an unsupported prototype. Kudos!
Oct 8 2016
Hrrm -- what is max_allowed_packet set to in MySQL?
How about this, it's solved or not?
This helped! Thanks for another fast fix.
Oct 7 2016
I have a patch to Arc that will use a non-printable control character (INFORMATION SEPARATOR THREE) instead of | for the separator.
T11747 wants a more advanced version of this (reviewing diffs between rendered documents), although the specific document type it wants (IPython notebooks) is likely very complex to render.
I'm going to merge this into T5698. That isn't quite the same task, but would be the first step toward this.
I believe this should now be fixed in HEAD of master. It should promote to stable within 24 hours.
T11744 has a vaguely-related special case where "split + count" was unreasonably slow for very large inputs.
D16683 makes us bail early if the combined length of the old and new lines is larger than 1280 bytes, which is a crude test for approximately two reasonable-length lines of emoji source code.
It looks like this is largely just PHP being PHP. To handle UTF8 combining characters correctly, we're producing a list of strings like this internally as the atoms of comparison:
This should now be fixed in HEAD of master.
In T3353#196761, @chad wrote:Can you do that today with view and edit policy?
I'll look at this tomorrow but can probably figure it out from this. Initial guess is that we're doing this (which is O(big constant * string length)):
Thanks also for the tip about SIGHUP to get a backtrace! Had not known that, and it looks extremely useful.
Just my 2c, but we've seen "table needs repair" on such a regular interval over the past 2-3 years of using Phabricator that our startup script includes a call to MySQL to automatically run "REPAIR TABLE" on startup. It's actually part of the Docker image because a broken MyISAM table will prevent bin/storage upgrade from running:
Oct 6 2016
Sweet -- thanks for the fast fix! I'll try applying it locally to clear out the affected tasks we have, and to make sure any new ones don't get me paged tonight. :-)
The root issue was that we would compute a ProseEditDistanceMatrix (roughly, a Levenshtein distance matrix) for large inputs, but this is O(N*M) in the size of the inputs and both of these inputs had ~2,200 "atoms" at the top prose diff level, so the matrix had ~5M elements.
Oh, one diagnostic tool that I didn't mention is that you can send SIGHUP to a running bin/* command to get a stacktrace of what it's doing dumped to /tmp/phabricator_backtrace_<pid> (usually). This one looks like it is a smoking gun:
Thank you! Not every day someone invites me to cause a crash on their production server. :-D