Stop handling 304s for comments pages/userpages

We maintain a system that (poorly) tries to decide if it should send a
304 for logged-out users on comments pages and userpages. There are a
lot of errors in the logic for this system that's causing us to not send
304s in cases that we should, and send them in cases where we shouldn't.

Instead of trying to fix all the errors in this system, we're just going
to remove it and leave that up to the CDN. I performed a test on
2015-09-14 by completely disabling the system that confirmed that it
could be removed without significant negative impact to the site.

There will be behavior changes to /r/friends as a result of this.
Previously, we were taking advantage of the timestamps stored by this
system to try to figure out which 100 friends (out of the 500 most
recently added to friends list) had most recently interacted with the
site, and pull posts from those users for /r/friends. Since we no
longer have these timestamps, this has been switched to just pick 100
friends at random every half hour, similar to how subreddits are
selected for a user's front page.
This commit is contained in:
Chad Birch
2015-09-14 17:05:21 -06:00
parent 7eb350591d
commit c155eeee84
10 changed files with 21 additions and 180 deletions

View File

@@ -52,7 +52,6 @@ from r2.lib.utils import (
query_string,
randstr,
sanitize_url,
set_last_modified,
timeago,
timefromnow,
timeuntil,
@@ -90,7 +89,6 @@ from r2.lib.pages.things import (
hot_links_by_url_listing,
wrap_links,
)
from r2.models.last_modified import LastModified
from r2.lib.menus import CommentSortMenu
from r2.lib.captcha import get_iden
@@ -1892,10 +1890,6 @@ class ApiController(RedditController):
ignore_missing=True,
)))
if kind == 'link':
set_last_modified(item, 'comments')
LastModified.touch(item._fullname, 'Comments')
wrapper = default_thing_wrapper(expand_children = True)
jquery(".content").replace_things(item, True, True, wrap = wrapper)
jquery(".content .link .rank").hide()
@@ -2910,16 +2904,6 @@ class ApiController(RedditController):
banner=c.user.name,
train_spam=train_spam)
modified_thing = None
if isinstance(thing, Link):
modified_thing = thing
elif isinstance(thing, Comment):
modified_thing = Link._byID(thing.link_id)
if modified_thing:
set_last_modified(modified_thing, 'comments')
LastModified.touch(modified_thing._fullname, 'Comments')
if isinstance(thing, (Link, Comment)):
sr = thing.subreddit_slow
action = 'remove' + thing.__class__.__name__.lower()

View File

@@ -259,9 +259,6 @@ class FrontController(RedditController):
is_embed = embeds.prepare_embed_request(sr)
# check for 304
self.check_modified(article, 'comments')
if is_embed:
embeds.set_up_embed(sr, comment, showedits=showedits)

View File

@@ -802,26 +802,21 @@ class UserController(ListingController):
def query(self):
q = None
if self.where == 'overview':
self.check_modified(self.vuser, 'overview')
q = queries.get_overview(self.vuser, self.sort, self.time)
elif self.where == 'comments':
self.check_modified(self.vuser, 'commented')
q = queries.get_comments(self.vuser, self.sort, self.time)
elif self.where == 'submitted':
self.check_modified(self.vuser, 'submitted')
q = queries.get_submitted(self.vuser, self.sort, self.time)
elif self.where == 'gilded':
self.check_modified(self.vuser, 'gilded')
if self.show == 'given':
q = queries.get_user_gildings(self.vuser)
else:
q = queries.get_gilded_user(self.vuser)
elif self.where in ('upvoted', 'downvoted'):
self.check_modified(self.vuser, self.where)
if self.where == 'upvoted':
q = queries.get_liked(self.vuser)
else:
@@ -837,7 +832,6 @@ class UserController(ListingController):
q = queries.get_saved(self.vuser, sr_id,
category=self.savedcategory)
elif self.where == 'actions':
self.check_modified(self.vuser, 'actions')
if not votes_visible(self.vuser):
q = queries.get_overview(self.vuser, self.sort, self.time)
else:

View File

@@ -1804,38 +1804,6 @@ class RedditController(OAuth2ResourceController):
body_parts.insert(1, script)
response.content = "".join(body_parts)
def check_modified(self, thing, action):
# this is a legacy shim until the old last_modified system is dead
last_modified = utils.last_modified_date(thing, action)
return self.abort_if_not_modified(last_modified)
def abort_if_not_modified(self, last_modified, private=True,
max_age=timedelta(0),
must_revalidate=True):
"""Check If-Modified-Since and abort(304) if appropriate."""
if c.user_is_loggedin:
return
# HTTP timestamps round to nearest second. truncate this value for
# comparisons.
last_modified = last_modified.replace(microsecond=0)
date_str = http_utils.http_date_str(last_modified)
response.headers['last-modified'] = date_str
cache_control = []
if private:
cache_control.append('private')
cache_control.append('max-age=%d' % max_age.total_seconds())
if must_revalidate:
cache_control.append('must-revalidate')
response.headers['cache-control'] = ', '.join(cache_control)
modified_since = request.if_modified_since
if modified_since and modified_since >= last_modified:
abort(304, 'not modified')
def search_fail(self, exception):
errpage = pages.RedditError(_("search failed"),
strings.search_failed)

View File

@@ -53,7 +53,6 @@ from r2.lib.utils import (
TimeoutFunction,
TimeoutFunctionException,
lowercase_keys_recursively,
set_last_modified,
timeinterval_fromstr,
tup,
)
@@ -973,20 +972,13 @@ class RuleTarget(object):
)
# TODO: shouldn't need to do all of this here
modified_thing = None
log_action = None
if isinstance(item, Link):
modified_thing = item
log_action = "removelink"
elif isinstance(item, Comment):
modified_thing = data["link"]
log_action = "removecomment"
queries.unnotify(item)
if modified_thing:
set_last_modified(modified_thing, "comments")
LastModified.touch(modified_thing._fullname, "Comments")
if log_action:
if self.action_reason:
reason = replace_placeholders(

View File

@@ -28,7 +28,7 @@ from r2.lib.db.thing import Thing, Merge
from r2.lib.db.operators import asc, desc, timeago
from r2.lib.db.sorts import epoch_seconds
from r2.lib.db import tdb_cassandra
from r2.lib.utils import fetch_things2, tup, UniqueIterator, set_last_modified
from r2.lib.utils import fetch_things2, tup, UniqueIterator
from r2.lib import utils
from r2.lib import amqp, filters
from r2.lib.comment_tree import add_comments, update_comment_votes
@@ -1871,28 +1871,6 @@ def handle_vote(user, thing, vote, foreground=False, timer=None, date=None):
new_vote(v, foreground=foreground, timer=timer)
timestamps = []
if isinstance(thing, Link):
#update the modified flags
if user._id == thing.author_id:
timestamps.append('Overview')
timestamps.append('Submitted')
elif isinstance(thing, Comment):
#update last modified
if user._id == thing.author_id:
timestamps.append('Overview')
timestamps.append('Commented')
else:
raise NotImplementedError
for timestamp in timestamps:
set_last_modified(user, timestamp.lower())
LastModified.touch(user._fullname, timestamps)
timer.intermediate("last_modified")
def process_votes(qname, limit=0):
# limit is taken but ignored for backwards compatibility

View File

@@ -1,61 +0,0 @@
# The contents of this file are subject to the Common Public Attribution
# License Version 1.0. (the "License"); you may not use this file except in
# compliance with the License. You may obtain a copy of the License at
# http://code.reddit.com/LICENSE. The License is based on the Mozilla Public
# License Version 1.1, but Sections 14 and 15 have been added to cover use of
# software over a computer network and provide for limited attribution for the
# Original Developer. In addition, Exhibit A has been modified to be consistent
# with Exhibit B.
#
# Software distributed under the License is distributed on an "AS IS" basis,
# WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License for
# the specific language governing rights and limitations under the License.
#
# The Original Code is reddit.
#
# The Original Developer is the Initial Developer. The Initial Developer of
# the Original Code is reddit Inc.
#
# All portions of the code written by reddit are Copyright (c) 2006-2015 reddit
# Inc. All Rights Reserved.
###############################################################################
from datetime import datetime
from utils import tup
import pytz
def make_last_modified():
last_modified = datetime.now(pytz.timezone('GMT'))
last_modified = last_modified.replace(microsecond = 0)
return last_modified
def last_modified_key(thing, action):
return 'last_%s_%s' % (str(action), thing._fullname)
def last_modified_date(thing, action, set_if_empty = True):
"""Returns the date that should be sent as the last-modified header."""
from pylons import app_globals as g
cache = g.permacache
key = last_modified_key(thing, action)
last_modified = cache.get(key)
if not last_modified and set_if_empty:
#if there is no last_modified, add one
last_modified = make_last_modified()
cache.set(key, last_modified)
return last_modified
def set_last_modified(thing, action):
from pylons import app_globals as g
key = last_modified_key(thing, action)
g.permacache.set(key, make_last_modified())
def last_modified_multi(things, action):
from pylons import app_globals as g
cache = g.permacache
things = tup(things)
keys = dict((last_modified_key(thing, action), thing) for thing in things)
last_modified = cache.get_multi(keys.keys())
return dict((keys[k], v) for k, v in last_modified.iteritems())

View File

@@ -20,6 +20,8 @@
# Inc. All Rights Reserved.
###############################################################################
import random
from r2.config import feature
from r2.lib.db.thing import Thing, Relation, NotFound
from r2.lib.db.operators import lower
@@ -447,6 +449,14 @@ class Account(Thing):
rel.note = note
rel._commit()
@memoize("get_random_friends", time=30*60)
def get_random_friends(self, limit=100):
friends = self.friend_ids()
if len(friends) > limit:
friends = random.sample(friends, limit)
return friends
def delete(self, delete_message=None):
self.delete_message = delete_message
self.delete_time = datetime.now(g.tz)

View File

@@ -22,7 +22,6 @@
from r2.lib.db import tdb_cassandra
from r2.lib import utils
from r2.models.last_modified import LastModified
from r2.models.link import Comment
from pycassa import batch, types
@@ -488,8 +487,6 @@ class CommentTree:
def add_comments(self, comments):
impl = self.IMPLEMENTATIONS[self.link.comment_tree_version]
impl.add_comments(self, comments)
utils.set_last_modified(self.link, 'comments')
LastModified.touch(self.link._fullname, 'Comments')
def add_comment(self, comment):
return self.add_comments([comment])

View File

@@ -53,13 +53,14 @@ from r2.lib.errors import UserRequiredException, RedditError
from r2.lib.geoip import location_by_ips
from r2.lib.memoize import memoize
from r2.lib.permissions import ModeratorPermissionSet
from r2.lib.utils import tup, last_modified_multi, fuzz_activity, \
unicode_title_to_ascii
from r2.lib.utils import (
timeago,
summarize_markdown,
in_chunks,
UrlParser,
fuzz_activity,
in_chunks,
summarize_markdown,
timeago,
tup,
unicode_title_to_ascii,
)
from r2.lib.cache import sgm
from r2.lib.strings import strings, Score
@@ -74,7 +75,6 @@ from r2.lib import hooks
from r2.models.query_cache import MergedCachedQuery
import pycassa
from r2.lib.utils import set_last_modified
from r2.models.keyvalue import NamedGlobals
from r2.models.wiki import WikiPage
import os.path
@@ -1526,31 +1526,13 @@ class FriendsSR(FakeSubreddit):
name = 'friends'
title = 'friends'
@classmethod
@memoize("get_important_friends", 5*60)
def get_important_friends(cls, user_id, max_lookup = 500, limit = 100):
a = Account._byID(user_id, data = True)
# friends are returned chronologically by date, so pick the end of the list
# for the most recent additions
friends = Account._byID(a.friends[-max_lookup:], return_dict = False,
data = True)
# only include friends that have ever interacted with the site
last_activity = last_modified_multi(friends, "overview")
friends = [x for x in friends if x in last_activity]
# sort friends by most recent interactions
friends.sort(key = lambda x: last_activity[x], reverse = True)
return [x._id for x in friends[:limit]]
def get_links(self, sort, time):
from r2.lib.db import queries
if not c.user_is_loggedin:
raise UserRequiredException
friends = self.get_important_friends(c.user._id)
friends = c.user.get_random_friends()
if not friends:
return []
@@ -1573,8 +1555,7 @@ class FriendsSR(FakeSubreddit):
if not c.user_is_loggedin:
raise UserRequiredException
friends = self.get_important_friends(c.user._id)
friends = c.user.get_random_friends()
if not friends:
return []
@@ -1597,7 +1578,8 @@ class FriendsSR(FakeSubreddit):
if not c.user_is_loggedin:
raise UserRequiredException
friends = self.get_important_friends(c.user._id)
friends = c.user.get_random_friends()
if not friends:
return []