768136 - Garbage collect marketplace

Status: RESOLVED FIXED

(Marketplace Graveyard :: Code Quality, defect)

Description

[Rob Hudson [:robhudson]]

AMO has a garbage collection cron:
https://github.com/mozilla/zamboni/blob/master/apps/amo/cron.py#L29

ActivityLogs now are settings based to append a '_mkt' to the tables. Either we add a new cron to run this same script so the '_mkt' tables are GC'd, or we set up marketplace with its own garbage collection cron with the AMO stuff stripped out.

Comment 1

[Wil Clouser [:clouserw]]

We can add the _mkt tables to the current function

Comment 2

[Andy McKay]

Running the exact same cron with a different settings file, eg: settings_local_mkt will do the job. Presuming that the job only touches the _mkt tables, or running it on the same tables multiple times won't hurt them.

Comment 3

[JR Conlin [:jrconlin,:jconlin]]

Digging into the code, I was about to suggest this. It appears that all the accessor code is wrapped by ActivityLog and like classes. Looking into the data to confirm that the various *_SUFFIX settings are applied the appropriate tables. (Since this will also give me a better idea of how the data is structured in marketplace.)

Comment 4

[Wil Clouser [:clouserw]]

That makes life easy then.  This bug is just a matter of adding a new line to https://github.com/mozilla/zamboni/blob/master/scripts/crontab/crontab.tpl then

Comment 5

[Rob Hudson [:robhudson]]

(In reply to Andy McKay [:andym] from comment #2)
> Presuming that the job only touches the
> _mkt tables, or running it on the same tables multiple times won't hurt them.

The AMO cron does a lot. It's probably ok, but it might be worthwhile to copy the GC over to mkt and strip out everything except the ActivityLog stuff. If there's market specific GC we need later it can go in the mkt GC. Anything shared would be picked up by the AMO GC.

Comment 6

[JR Conlin [:jrconlin,:jconlin]]

My only concern here is that there would be two commands, gc and gc_* where * is whatever sub-application. There's already at least one, "mkt". 

Since there is strong discussion to eventually cleanly separate "market" from "amo" then this may not be an issue. If we are not planning on separating the two, then we should try and keep the code similar, and control this via configuration or other product specific means.

Comment 7

[Rob Hudson [:robhudson]]

I can't speak to the future plans of market and AMO, so I'll defer to someone else on that. I think we can run with your pull request for now... if there's any fallout from running the GC twice, we can fix it later.

Comment 8

[Wil Clouser [:clouserw]]

Long term pla is still to merge AMO into the marketplace and have only marketplace.m.o.

Comment 9

[Christopher Van Wiemeersch [:cvan]]

https://github.com/mozilla/zamboni/commit/a6b6db6