638257 - Script to purge expired ttls from the databases

Status: RESOLVED FIXED

(Cloud Services :: Server: Other, defect, P2)

Description

[Toby Elliott [:telliott]]

Attachment: Script to purge old ttls

Uses the node table and the cluster config file to figure out what to delete from.

Comment 1

[Tarek Ziadé (:tarek)]

Comment on attachment 516420 [details]
Script to purge old ttls

...

unless this is a use-once-and-throw script: is there any good reason here not to use the SQLAlchemy SQL expression language + engine here ?

The benefits are:

+++ you can actually test this code by running tests against a sqlite DB
++ you don't have to escape the values and build the query manually like you do here
+ the script can work directly with our sqluri configs, and we're not bound to the MySQLdb driver, meaning that this admib script can be reused on any Sync server out there (there's one running under postgres for instance)



>def purge_ttl(host, db, user, password, tablename, timestamp):
>    sql = "delete from %s where ttl < %i" % (tablename, timestamp)
>    try:
>        conn = MySQLdb.connect (host = host, user = user,
>                                passwd = password, db = db)
>        cursor = conn.cursor()
>        cursor.execute(sql)
>        cursor.close()
>    except MySQLdb.Error, e:
>         logging.error("Error %d: %s" % (e.args[0], e.args[1]))
>         return False
>    return True
>
...

Comment 2

[Tarek Ziadé (:tarek)]

Comment on attachment 516420 [details]
Script to purge old ttls

Found a small issue on a second read:

>    storage = AdminStorage(config.get('syncnodes', 'admin_uri'))
>    for row in storage.get_values(cluster, node):
>        if row['db_modulo'] > 1:
>            for modulo in range(row['db_modulo']):
>                tablename = table + "%0d" % modulo
>                if not dryrun:
>                    purge_ttl(row['db_host'], row['db_name'],
>                              cluster_config.get(cluster, 'db_user'),
>                              cluster_config.get(cluster, 'db_password'),
>                              tablename, timestamp)
>                print "did %s (%s)" % (row['node'], tablename)
>        else:

Shouldn't 'dryrun' be tested here too ?

>            purge_ttl(row['db_host'], row['db_name'],
>                      cluster_config.get(cluster, 'db_user'),
>                      cluster_config.get(cluster, 'db_password'),
>                      table, timestamp)
>            print "did %s" % row['node']


You should remove code redundancy here like this to avoid any cut-n-paste issue:

if row['db_modulo'] > 1:
   range_ = range(row['db_modulo'])
else:
   range_ = [1]

for modulo in range_:
   tablename = table + "%0d" % modulo
   if not dryrun:
       purge_ttl(row['db_host'], row['db_name'],
                 cluster_config.get(cluster, 'db_user'),
                 cluster_config.get(cluster, 'db_password'),
                 tablename, timestamp)
   print "did %s (%s)" % (row['node'], tablename)

Comment 3

[Tarek Ziadé (:tarek)]

s/range_ = [1]/range_ = [0]/

Comment 4

[Tarek Ziadé (:tarek)]

hum, did it too fast -- here, much better:

if row['db_modulo'] > 1:
   tables = [table + "%0d" % modulo for modulo in range(row['db_modulo'])]
else:
   tables = [table]

for table in tables:
   if not dryrun:
       purge_ttl(row['db_host'], row['db_name'],
                 cluster_config.get(cluster, 'db_user'),
                 cluster_config.get(cluster, 'db_password'),
                 table, timestamp)
   print "did %s (%s)" % (row['node'], table)

Comment 5

[Toby Elliott [:telliott]]

Setting up SQLAlchemy for a mysql 1-liner seems like a bunch of overhead. This is going to run on a server that doesn't have sync on it.

Comment 6

[Tarek Ziadé (:tarek)]

(In reply to comment #5)
> Setting up SQLAlchemy for a mysql 1-liner seems like a bunch of overhead. This
> is going to run on a server that doesn't have sync on it.

So, in what repository this script is going to land ?

Comment 7

[Toby Elliott [:telliott]]

I'm thinking server-node-assignment at the moment, since it does use that. The other possibility is admin-scripts, but I'd like to spend some time rethinking how we use that.

Comment 8

[Tarek Ziadé (:tarek)]

What's the status on this issue ?

Comment 9

[Toby Elliott [:telliott]]

Attachment: revised script for purging ttls

Philip has indicated this is a higher priority now, so pushing this forwards a bit.

Comment 10

[Tarek Ziadé (:tarek)]

Comment on attachment 535434 [details] [diff] [review]
revised script for purging ttls

codes looks good. 

I think having a sample sqlite DB in a test to check that the script does what's wanted could be good to have for future changes here

Comment 11

[Toby Elliott [:telliott]]

I believe this code is now in production, so closing.