This doesn't remove the existing loid generation in JS, but will supercede it as the first request from the server will have the an loid cookie.
This also doesn't change the current cookie behavior, but rather localizes all of the loid generation code to a module so that we can eventually fix it up and not break behavior in the rest of the app.
The CommentBuilder has 3 different modes:
* regular mode for building a comments page
* permalink mode for building the tree starting from a specific comment
* morechildren mode for building the tree starting from specific comments
num_children is derived directly from the tree, so it makes sense
to calculate it on CommentTree retrieval rather than waiting and making
the CommentBuilder handle it. Calculating child counts in advance for the
full tree appears to be faster than the previous method of using
`get_num_children()` to calculate the child count only for visible comments.
This is normally caught by the VUname validator, but simultaneous
requests can both pass that check and then the second one will trigger
the exception when actually trying to create the account.
Adds images to /about/stylesheets for easy view, even when a non mod, since the images were already accessible via inspect element as well as via the JSON template, as semi-wanted on /r/csshelp numerous times.
Using background threads causes annoying complications in general (like right
now if the worker thread is backed up but we restart the app we just lose the
work). But the particular problem at hand is that the permacache zlib patch
wants to do compression. That can cost a lot of CPU so we want to know how long
it takes which means that we need timers. But timers work by reading from
g.stats, and g isn't available in the background thread.
We could find some other way to do the timers, but I'd rather just stop using
the AMQP background thread for anything but actual AMQP stuff which will kill
both birds with one stone.
At a glance it looks like this would cost us roughly 25ms per update to these
listings. 99th percentile goes up to 480ms but seems to float nearer to 100ms.
Deletes has a crazier 99th percentile but a similar mean. Way fewer of them
happen.
**Explanation**: compute_time_listings is slow. Really slow. At a quick glance, here are the jobs running right now:
date: Sun Jan 17 20:04:56 PST 2016
-rw-rw-r-- 1 ri ri 1.2G Jan 17 12:37 comment-week-data.dump
-rw-rw-r-- 1 ri ri 683M Jan 17 12:25 comment-week-thing.dump
-rw-rw-r-- 1 ri ri 53G Jan 16 07:13 comment-year-data.dump
-rw-rw-r-- 1 ri ri 31G Jan 16 04:37 comment-year-thing.dump
-rw-rw-r-- 1 ri ri 276M Jan 17 17:04 link-week-data.dump
-rw-rw-r-- 1 ri ri 70M Jan 17 17:03 link-week-thing.dump
So the currently running top-comments-by-year listing has been running for nearly 37 hours and isn't done. top-comments-by-week has been running for 8 hours. top-links-by-week has been running for 3 hours. And this is just me checking on currently running jobs, not actual completion times.
The slow bit is the actual writing to Cassandra in `write_permacache`. This is mostly because `write_permacache` is extremely naive and blocks waiting for individual writes with no batching or parallelisation. There are a lot of ways to work around this and some of them will become easier when we're not longer writing out to the permacache at all, but until then (and even after that) this approach lets us keep doing the simple-to-understand thing while parallelising some of the work.
**The approach**: `compute_time_listings` is written as a mapreduce job in our `mr_tools` toolkit, with `write_permacache` as the final reducer. In `mr_tools`, you can run multiple reducers as long as a given reducer can be guaranteed to receive all of the keys for the same key. So this patch adds `hashdist.py`, a tool that runs multiple copies of a target job and distributes lines to them from stdin using their first tab-delimited field to meet this promise. (The same script could apply to mappers and sorts too but in my tests for this job the gains were minimal because `write_permacache` is still the bottleneck up to a large number of reducers.)
**Numbers**: A top-links-by-hour listing in prod right now takes 1m46.387s to run. This patch reduces that to 0m43.960s using 2 jobs (a 60% savings). That top-links-by-week job that before I killed after 3 hours completed in 56m47.329s. The top-links-by-year job that I killed last week at over 36 hours finished in 19 hours.
**Downsides**: It costs some additional RAM: roughly 10mb for hashdist.py and 100mb in memory for each additional copy of the job. It multiplies the effective load on Cassandra by the number of jobs (although I have no reason to believe that it's practical to overload Cassandra this way right now; I've tested up to 5 jobs).
**Further work**: with this we could easily do sort|reducer fusion to significantly reduce the work required by the sorter. `hashdist.py` as written is pretty slow and is only acceptable because `write_permcache` is even slower; a non-Python implementation would be straight forward and way faster.
`add_queries` occurs in the AMQP worker thread by default. The results of
https://github.com/reddit/reddit-public/pull/4687 will tell us if that's a good
idea in general, but in the mean time it probably doesn't make sense to do it
for queue processes
In the permacachezlib patch we want to be able to time how long we're spending
doing the compression and serialisation. There are times that we use the
permacache in the AMQP worker thread, which can't get to g, so it can't get to
g.stats. The solution is probably to just stop using the permacache in this way,
but first we'll want to know what it would cost us to start doing these
operations in the foreground.
Most permacache mutations occur in queue processes where it shouldn't matter,
but there are cases like deletions and inboxes where they occur in-request. So
knowing the costs of foregrounding them is important.
Note that this causes the timers to occur in the worker thread. That has known
to be safe in the past, but it's been a while since the support was used.
By default, Linux's sort(1) uses locale-based sorting. Normally this is what
humans want, but for mapreduce it breaks the guarantee that the same reducer
always sees each instance of the same key. Here's an example:
user/comment/top/week/1102922 26 1453516098.92 t1_cz8jgq9
user/comment/top/week/1102922 3 1453527927.97 t1_cz8ovzj
user/comment/top/week/11029224 1 1453662674.45 t1_cza98tb
user/comment/top/week/1102922 4 1453515976.97 t1_cz8jee8
user/comment/top/week/1102922 4 1453519790.67 t1_cz8lavb
user/comment/top/week/11029224 2 1453827188.31 t1_czcotf1
user/comment/top/week/1102922 7 1453521946.74 t1_cz8mb50
user/comment/top/week/1102922 7 1453524230.93 t1_cz8ncj2
user/comment/top/week/1102922 7 1453527760.32 t1_cz8otkx
user/comment/top/week/1102922 7 1453528700.96 t1_cz8p6u3
user/comment/top/week/11029228 1 1453285875.44 t1_cz525gu
user/comment/top/week/11029228 1 1453292202.65 t1_cz53ulm
user/comment/top/week/11029228 1 1453292232.55 t1_cz53uxe
According to sort(1) using the default locale, this is already sorted.
Unfortunately, that means that to a reducer this list represents 6 different
listings (each of which will overwrite the previous runs of the same listing).
But that's not what we want. It's actually two listings, like:
user/comment/top/week/1102922 26 1453516098.92 t1_cz8jgq9
user/comment/top/week/1102922 3 1453527927.97 t1_cz8ovzj
user/comment/top/week/1102922 4 1453515976.97 t1_cz8jee8
user/comment/top/week/1102922 4 1453519790.67 t1_cz8lavb
user/comment/top/week/1102922 7 1453521946.74 t1_cz8mb50
user/comment/top/week/1102922 7 1453524230.93 t1_cz8ncj2
user/comment/top/week/1102922 7 1453527760.32 t1_cz8otkx
user/comment/top/week/1102922 7 1453528700.96 t1_cz8p6u3
user/comment/top/week/11029224 1 1453662674.45 t1_cza98tb
user/comment/top/week/11029224 2 1453827188.31 t1_czcotf1
user/comment/top/week/11029228 1 1453285875.44 t1_cz525gu
user/comment/top/week/11029228 1 1453292202.65 t1_cz53ulm
user/comment/top/week/11029228 1 1453292232.55 t1_cz53uxe
To do this, we need to set the enviroment variable LC_ALL=C when running sort(1)
to indicate that the sorting should operate only on the raw bytes.
It looks like this has been broken since the Trusty Tahr upgrade.
This is to resolve the problem that we can generate markdown with named entities
like ` `. These are valid in XHTML's DTD but because entities aren't
namespaced they aren't valid in Atom documents, even within the XHTML namespace.
The fix is to switch to the RSS style double-escaped HTML type for `<content>`
blocks
Another approach would be to used numeric entities instead, but since we allow
users to type HTML entities we'd have to replace them too. That's a pretty large
change for such a small problem.
As a side effect of this approach, this means we also have to get rid of the
"[23 comments]" label on Link's feed items because the double-escaping breaks
the StringTemplate replacement
We have our Mako filters set to escape HTML by default. Unfortunately RSS
requires double escaping in some places and not in others, so there isn't a
reasonable default. Here I have done a pass through the `.xml` templates to find
user data that's ending up single-escaped and added double escaping to them.
This requirement is because RSS grew HTML support organically in a way that
clients can't tell if a field actually contains HTML or not. Sometimes it's
double escaped, sometimes it's not. Clients have to take a guess by sniffing for
`>` characters and hoping they get it right. This also means that it's
impossible for servers to reliably tell the clients which data this field
contains.
This is a bit of a ticking time bomb. Users may find ways to sneak in HTML in
the date field, or we may add new templates that forget to do double escaping on
little-used fields. I recommend that we switch these to use Atom which always
indicates whether the fields contain HTML or not. Work has started on this
conversion in another branch.
* ref https://www.reddit.com/r/AskNetsec/comments/41larg/titleheadbody_idmsgfeedsummarybodyimg/
* ref https://reddit.atlassian.net/browse/INFRA-721
* ref https://bugzilla.mozilla.org/show_bug.cgi?id=1240603
The cause of the bug is that if we fail to start because someone else has already started, we still delete their files.
From job-02 right now:
write_permacache() # comment ("day", "week+
write_permacache() # link ("day","week")
write_permacache() # link ("day","week")
write_permacache() # comment ("day", "week+
write_permacache() # link ("month","year")
write_permacache() # comment ("month", "ye+
