Workaround for the json migration caused by #407

[aqeelat]

We currently have more than 71,490,242 log entry records. This migration is going to be really slow for us and could break our application.

Usually, when we have a slow non-blocking migration script, we either run it as SQL directly on the DB or create a python script and run it in a Django shell on a separate pod.

I'm currently thinking of a way we can apply the same approach to the JSON migration.

The flow I'm thinking of is:

1. run a pre-migration SQL script to:
   1. rename the current changes column to "old_changes"
   2. create a new changes column
2. Run migration 0015
3. run a post-migration SQL script to:
   1. run the Modify change field to be a json field. #407 (comment) scripts on the old column
   2. copy the data from the old column to the new column
   3. delete the old column

To make this transition smooth, maybe we can update the migration file (0015) by prepending it with a migrations.RunPython that only runs the pre-migration script if the conf AUDITLOG_ASYNC_JSON_MIGRATION is set to True.

Then the dev can then be responsible for running step 3 (or, optionally, we can add it as a management command)

What do you think?
@hramezani @alieh-rymasheuski

[aleh-rymasheuski]

Vote for a management command and two-step upgrade process:

Step 1:

1. changes is renamed to _changes_text.
2. A new JSON column changes is added.
3. A model-level accessor for changes is added which will effectively do return self.changes or json.loads(self._changes_text).
4. A management command is added which copies (moves?) the data from _changes_text to the new changes column for all historical records, batch by batch, focusing on the availability of the app.

Step 2:

1. _changes_text is removed, together with the accessor and the management command.

Step 1 should be considered a breaking change. Step 2 should not be considered a breaking change because it will just affect the implementation of the model which is not public API. Step 2 should be safeguarded by a conf variable like AUDITLOG_I_SOLEMNLY_SWEAR_THAT_I_MIGRATED_THE_HISTORICAL_DATA (dunno how to make it both required for existing installations and not intrusive for the new users - maybe call it something smart), so that no users will drop their historical data unknowingly.

[aqeelat]

@alieh-rymasheuski in your suggestion, would the steps be separate versions or one version but controller by a conf variable?

[aqeelat]

@alieh-rymasheuski

1. IMO, the main concern is collecting the logs. Reading the logs could wait until the migration is complete.
2. I suggest naming the variable something like AUDITLOG_READ_FROM_TEXT_CHANGES. Then we do something like the code below.
3. I think the management command can do:
   1. if the column _changes_text exists and has non-null values, then jsonify the values
   2. if the column _changes_text exists and does not have any values, then drop the column
   3. else, return None.

def default_get_changes():
    return self.changes

def get_changes_or_text():
    if self.changes:
        return self.changes
    elif self._text_changes:
        return json.dumps(self._text_changes)
    return None

if conf.AUDITLOG_READ_FROM_TEXT_CHANGES:
    get_changes =  get_changes_or_text
else:
    get_changes = default_get_changes

[aleh-rymasheuski]

would the steps be separate versions or one version but controller by a conf variable?

Separate versions.

[hramezani]

@aqeelat Is it possible for you to create a copy/replica database from your current database and try to run the migration on it to see how long does it take?

[aleh-rymasheuski]

IMO, the main concern is collecting the logs

@aqeelat, IMO the main concern is availability of the database and/or the application for the duration of the JSON migration. A bigger service in my herd has over 100M auditlog entries, so running this migration synchronously won't be viable for this service. Last time I had to add a new index to the auditlog table on this service that operation took multiple hours, and the table was substantially smaller back then.

The setting I propose will guard an inattentive user from dropping the column without running the (asynchronous) data migration script. I'm not at all concerned about reading the logs while the migration is partially done, the accessor property I described should protect the users from the moment they deploy Step 1 version and until they have a suitable moment to run the data migration script.

I don't insist that the project follows my suggestion. After all, I'm using a fork of django-auditlog and, if needed, I'll script my own data migration process in case if we decide on keeping the upstream simple. :)

[aqeelat]

@hramezani I told my SRE team to create a replica and run it, and it timed-out after 1 minute. So, it does take longer than we're comfortable with.

[hramezani]

@aqeelat How did you run the migration? in a separate pod? where did you get the time out? request to DB?

IMO, The main problem here is the database availability.

[aqeelat]

We ran it directly from the dbms.

…

On Jan 9, 2023, at 2:01 PM, Hasan Ramezani ***@***.***> wrote: @aqeelat <https://github.com/aqeelat> How did you run the migration? in a separate pod? where did you get the time out? request to DB? IMO, The main problem here is the database availability. — Reply to this email directly, view it on GitHub <#490 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABHYUSAEIY2K7ELJLDJPXULWRPVZ3ANCNFSM6AAAAAATM2RJRM>. You are receiving this because you were mentioned.

[hramezani]

I think we can add a separate documentation page for migrating to 3.0.0 and write the suggestions there. Also, we can have a link to this issue there as well.
It might be hard to find a solution that covers all the cases. I think at the end people have to migrate their own data if they have a lot of records.

BTW, I am still open to accept a PR if it is good enough. for example having a management command ro do the migration.

[aqeelat]

@hramezani I'll propose something this week.

[aqeelat]

@hramezani @alieh-rymasheuski this is what I have in mind: #495

So, if developers want to opt in to the two-step migration (we can call it async migration), they need to set both new confs to True, and then after the migration, they can set AUDITLOG_USE_TEXT_CHANGES_IF_JSON_IS_NOT_PRESENT to True.

The next major version of auditlog should remove changes_text and the management command.

This way, new users and users who don't opt-in to this process won't notice anything other that the column in their db.

[aqeelat]

If you guys find this approach acceptable, I'll install mysql and oracle locally and get the sql migration command and add it to the PR.