Opened 4 years ago

Closed 4 years ago

#2983 closed defect (fixed)

Stats processing fails to interprete lastVersion parameter

Reported by: matze Assignee: trev
Priority: P1 Milestone:
Module: Sitescripts Keywords:
Cc: fhd, fred Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29329315/

Description (last modified by trev)

Background

Stats processing on stats1.adblockplus.org is currently failing consistently. The python -m sitescript.stats.bin.logprocessor command produces the following exception for several logs:

Traceback (most recent call last):
  File "/opt/sitescripts/sitescripts/stats/bin/logprocessor.py", line 530, in parse_source
    data = parse_fileobj(mirror_name, fileobj, geo, geov6, ignored)
  File "/opt/sitescripts/sitescripts/stats/bin/logprocessor.py", line 472, in parse_fileobj
    info = parse_record(line, ignored, geo, geov6)
  File "/opt/sitescripts/sitescripts/stats/bin/logprocessor.py", line 421, in parse_record
    parse_downloader_query(info)
  File "/opt/sitescripts/sitescripts/stats/bin/logprocessor.py", line 308, in parse_downloader_query
    last_update = parse_lastversion(last_version)
  File "/opt/sitescripts/sitescripts/stats/bin/logprocessor.py", line 119, in wrapped
    results.popitem(last=False)
  File "/usr/lib/pypy/lib-python/2.7/collections.py", line 162, in popitem
    raise KeyError('dictionary is empty')
KeyError: 'dictionary is empty'

Analysis

The problem is that parse_lastversion() fails to parse the new format for lastVersion (123456789-1/0 instead of 123456789) and throws an exception. This is an expected situation and we handle it, but it also messes up cache_lru decorator - it doesn't expect exceptions and removes an element from the cache but doesn't add one instead. As a result, the cache goes empty because it thinks that there are no free spots left.

Change History (6)

comment:1 Changed 4 years ago by trev

  • Owner set to trev

This error makes little sense so far unfortunately, that cache cannot be empty. I did verify that the cache algorithm is correct by code inspection, and I did run tests with random input. I will try running processing with real logs now...

comment:2 Changed 4 years ago by trev

Ok, the cache can be empty - if the processing function throws. And that's what it does, because it cannot process lastVersion parameter.

comment:3 Changed 4 years ago by trev

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:4 Changed 4 years ago by trev

  • Description modified (diff)

comment:5 Changed 4 years ago by trev

  • Description modified (diff)

comment:6 Changed 4 years ago by trev

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.