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+
* Make them lockless, because mr_top is the only one that ever writes to it.
This avoids a lot of memcached round trips
* Don't set them to permacache_memcaches. Delete from it instead.
This keeps us from blowing out that whole cache with listings that will never
be read out of every time mr_top runs.
* Fix a performance bug in _mr_tools.mr_reduce_max_per_key
timeouts.js and locked.js have pretty much the same code for managing
their popups. Pull this out into a file to reduce duplicate code, and
also make it easier to manage more of the same type of popup
If the cached query doesn't exist yet, "data" will be an empty list.
If we try to insert >= 1000 items, the chunk of code under the
"would_truncate" condition will run and raise "IndexError: list index
out of range"
Some account information for users in permanent suspension, such as karma, are
still accessible through the api. This info is hidden on the site, so it should
be treated similarly in the API response.
This needs to be called on posts being removed by the "all" filter
setting to update some info on them and put them into the correct cached
queries so that they show up in pages where the mods expect them.
You can run `vagrant up` from any directory within a project--vagrant
will look for a Vagrantfile in the current directory and if it doesn't
find one walk up one directory, and repeat until it finds a Vagrantfile.
Previously the `code_share_host_path` was set assuming that `vagrant up`
would always be run from the root reddit directory. Now it is set from the
fixed known location of the Vagrantfile.
This was previously being done in the spam-filter itself, so if there
are site issues causing the filter to fall behind, subreddits that use
the "all" setting to initially remove all posts would have it stop
working until the filter was able to catch up.
This moves the check into the actual posting process so that it isn't
dependent on the filter at all (and will also make this setting function
on open-source installs where the spam-filter code isn't available).
