Add limit CLI flags to prune jobs #655

Merged
floatingghost merged 11 commits from Oneric/akkoma:prune-batch into develop 2024-06-17 20:47:53 +00:00
Member

The prune tasks can incur heavy database load and take a long time, grinding the instance to an halt for the entire duration. The main culprit behind this is pruning orphaned activities, but that part is also quiet helpful so just omitting it completely is not an option.

This patch series makes pruning of orphaned activities available as a standalone task (prune_objects still keeps its --prune-orphaned-activities flag), optimises the “activities referring to an array of objects” case and adds a couple toggles to the new task to enable a more controlled and background frienedly cleanup if needed/desired.

This proved very useful for akko.wtf; without this a full prune run took several days during which the instance became unusable. Further down in this pr thread smitten also reported unpatched pruning could OOM-kill the instance on smaller VPS.

If your instance is (relative to your hardware) big enough for a single full prune to be problematic, a pruning session with the patches here could look like:

# add/remove whatever flags you like here as long as
#  --prune-orphaned-activities  and --vacuum are omitted.
# (_IF_ this is already struggling try also omitting --keep-threads
#  or adding the newly added --limit here and running it multiple times)
mix pleroma.database prune_objects --keep-thread

# with patches cleaning out array activities shouldn’t take
# too much resources and thus not need a limit
mix pleroma.database prune_orphaned_activities --no-singles

# script repeatedly running
#  mix pleroma.database prune_orphaned_activities --no-arrays --limit XXX
# as long as it completes within a set timeframe; see below
./batch_pruning.sh

Code for ./batch_pruning.sh:

#!/bin/sh

# Tweak this for your own setup!
# Larger values will be more efficient overall,
# but _too_ large will bog everything down again.
# Values tested for a ~70 monthly active users instance
# hosted on a 4 vCPU (Ryzen 7 2700X) 8GB RAM VM
YIELD=120
BATCH_SIZE=150000
BATCH_MAX_TIME=500

while : ; do
    start="$(date +%s)"
    out=$( \
        mix pleroma.database prune_orphaned_activities --no-arrays --limit "$BATCH_SIZE" \
        | grep -E ' (^|\] )Deleted ' \
        | tail -n 1 \
    )"
    end="$(date +%s)"
    duration="$((end - start))"
    echo "$out"

    if echo "$out" | grep -qE 'Deleted 0 rows$' ; then
        echo "Nothing more to delete."
        break
    elif echo "$out" | grep -qE 'Deleted [0-9]+ rows$' ;
        :
    else
        echo "Unexpected status report, abort! Expected count of total deleted rows, got:" >&2
        echo "    $out" >&2
        exit 2
    fi

    if [ "$duration" -gt "$BATCH_MAX_TIME" ] ; then
        echo "Completion of single batch takes too long ($duration > $BATCH_MAX_TIME)" >&2
        echo "Abort further batches to not bog down the instance!" >&2
        exit 1
    fi
    sleep "$YIELD"
done
Alternative version with dynamic batch size

Since initially when there are still many orphaned activities things will go quicker, using a dynamic batch size will be more efficient. But again, this needs tweaking for the needs and capabilities of your specific instance and hardware setup. Don't go too crazy with initial size though, else things will likely get bogged down or OOMed again.

#!/bin/sh

YIELD=120
BATCH_SIZE_MAX=250000
BATCH_SIZE_MIN=100000
BATCH_MAX_TIME=300

set -eu

# params: cur_batch_time cur_batch_size
# returns: new_batch_size (0 if constraints cannot be met; otherwise valid)
lower_batch_size() {
    # Intentional rounding imprecision to facillitate going _below_ max time
    div="$(( ($1 + BATCH_MAX_TIME - 1) / BATCH_MAX_TIME ))"
    newbatch="$(($2 / div))"
    if [ "$newbatch" -lt "$BATCH_SIZE_MIN" ] ; then
        newbatch=0
    fi
    echo "$newbatch"
}

BATCH_SIZE="$BATCH_SIZE_MAX"
echo "Starting with batch size $BATCH_SIZE"
while : ; do
    start="$(date +%s)"
    out="$( \
        mix pleroma.database prune_orphaned_activities --no-arrays --limit "$BATCH_SIZE" \
        | grep -E ''(^|\] )Deleted ' \
    )"
    end="$(date +%s)"
	duration="$((end - start))"
	echo "$out"

	if echo "$out" | tail -n 1 | grep -qE 'Deleted 0 rows$' ; then
		echo "Nothing more to delete."
		break
    elif echo "$out" | grep -qE 'Deleted [0-9]+ rows$' ;
        :
    else
        echo "Unexpected status report, abort! Expected count of total deleted rows, got:" >&2
        echo "    $out" >&2
        exit 2
    fi

	if [ "$duration" -gt "$BATCH_MAX_TIME" ] ; then
		echo "Completion of single batch takes too long ($duration > $BATCH_MAX_TIME)" >&2
        BATCH_SIZE="$(lower_batch_size "$duration" "$BATCH_SIZE")"
        if [ "$BATCH_SIZE" -gt 0 ] ; then
            echo "Try lowering batch size to $BATCH_SIZE..."
        else
    		echo "Cannot lower batch size further. Abort to not bog down instance!" >&2
		    exit 1
        fi
	fi
	sleep "$YIELD"
done

The problem with an unconstrained prune is that it will go through many millions of activites and objects, left-joins 4 tables which apart from users are all very large and often many of them will be eligible and not just filtered out early. This obviously takes long to process and can lead to such a large data stream and queued up changes by the delete transaction that as it goes on, after a while it bogs down everything else.
Splitting it up into batches limits how much data will be processed at once thus avoiding the problem and allowing pruning most or all eligible activities in the background or allow it to work at all on weaker hardware. (Though after enough activites were cleared out by limited batches an unconstrainted query might have become feasible again and the overhead of limited btaches significant, so if you really want to clean out everything consider if it’s possible to switch to an unlimtited prune at the end)

Best reviewed commit by commit and as noted in the commit messages, many of the diff lines are just indentation adjustments and for review its probably a good idea to hide whitespace-only changes.

Resolves #653 ; cc @norm

The prune tasks can incur heavy database load and take a long time, grinding the instance to an halt for the entire duration. The main culprit behind this is pruning orphaned activities, but that part is also quiet helpful so just omitting it completely is not an option. This patch series makes pruning of orphaned activities available as a standalone task (`prune_objects` still keeps its `--prune-orphaned-activities` flag), optimises the “activities referring to an array of objects” case and adds a couple toggles to the new task to enable a more controlled and background frienedly cleanup if needed/desired. This proved very useful for akko.wtf; without this a full prune run took several days during which the instance became unusable. Further down in this pr thread smitten also reported unpatched pruning could OOM-kill the instance on smaller VPS. If your instance is (relative to your hardware) big enough for a single full prune to be problematic, a pruning session with the patches here could look like: ```sh # add/remove whatever flags you like here as long as # --prune-orphaned-activities and --vacuum are omitted. # (_IF_ this is already struggling try also omitting --keep-threads # or adding the newly added --limit here and running it multiple times) mix pleroma.database prune_objects --keep-thread # with patches cleaning out array activities shouldn’t take # too much resources and thus not need a limit mix pleroma.database prune_orphaned_activities --no-singles # script repeatedly running # mix pleroma.database prune_orphaned_activities --no-arrays --limit XXX # as long as it completes within a set timeframe; see below ./batch_pruning.sh ``` Code for `./batch_pruning.sh`: ```sh #!/bin/sh # Tweak this for your own setup! # Larger values will be more efficient overall, # but _too_ large will bog everything down again. # Values tested for a ~70 monthly active users instance # hosted on a 4 vCPU (Ryzen 7 2700X) 8GB RAM VM YIELD=120 BATCH_SIZE=150000 BATCH_MAX_TIME=500 while : ; do start="$(date +%s)" out=$( \ mix pleroma.database prune_orphaned_activities --no-arrays --limit "$BATCH_SIZE" \ | grep -E ' (^|\] )Deleted ' \ | tail -n 1 \ )" end="$(date +%s)" duration="$((end - start))" echo "$out" if echo "$out" | grep -qE 'Deleted 0 rows$' ; then echo "Nothing more to delete." break elif echo "$out" | grep -qE 'Deleted [0-9]+ rows$' ; : else echo "Unexpected status report, abort! Expected count of total deleted rows, got:" >&2 echo " $out" >&2 exit 2 fi if [ "$duration" -gt "$BATCH_MAX_TIME" ] ; then echo "Completion of single batch takes too long ($duration > $BATCH_MAX_TIME)" >&2 echo "Abort further batches to not bog down the instance!" >&2 exit 1 fi sleep "$YIELD" done ``` <details> <summary><strong>Alternative version with dynamic batch size</strong></summary> Since initially when there are still many orphaned activities things will go quicker, using a dynamic batch size will be more efficient. But again, this needs tweaking for the needs and capabilities of your specific instance and hardware setup. Don't go too crazy with initial size though, else things will likely get bogged down or OOMed again. ```sh #!/bin/sh YIELD=120 BATCH_SIZE_MAX=250000 BATCH_SIZE_MIN=100000 BATCH_MAX_TIME=300 set -eu # params: cur_batch_time cur_batch_size # returns: new_batch_size (0 if constraints cannot be met; otherwise valid) lower_batch_size() { # Intentional rounding imprecision to facillitate going _below_ max time div="$(( ($1 + BATCH_MAX_TIME - 1) / BATCH_MAX_TIME ))" newbatch="$(($2 / div))" if [ "$newbatch" -lt "$BATCH_SIZE_MIN" ] ; then newbatch=0 fi echo "$newbatch" } BATCH_SIZE="$BATCH_SIZE_MAX" echo "Starting with batch size $BATCH_SIZE" while : ; do start="$(date +%s)" out="$( \ mix pleroma.database prune_orphaned_activities --no-arrays --limit "$BATCH_SIZE" \ | grep -E ''(^|\] )Deleted ' \ )" end="$(date +%s)" duration="$((end - start))" echo "$out" if echo "$out" | tail -n 1 | grep -qE 'Deleted 0 rows$' ; then echo "Nothing more to delete." break elif echo "$out" | grep -qE 'Deleted [0-9]+ rows$' ; : else echo "Unexpected status report, abort! Expected count of total deleted rows, got:" >&2 echo " $out" >&2 exit 2 fi if [ "$duration" -gt "$BATCH_MAX_TIME" ] ; then echo "Completion of single batch takes too long ($duration > $BATCH_MAX_TIME)" >&2 BATCH_SIZE="$(lower_batch_size "$duration" "$BATCH_SIZE")" if [ "$BATCH_SIZE" -gt 0 ] ; then echo "Try lowering batch size to $BATCH_SIZE..." else echo "Cannot lower batch size further. Abort to not bog down instance!" >&2 exit 1 fi fi sleep "$YIELD" done ``` -------- </details> The problem with an unconstrained prune is that it will go through many *millions* of activites and objects, left-joins 4 tables which apart from `users` are all very large and often many of them will be eligible and not just filtered out early. This obviously takes long to process and can lead to such a large data stream and queued up changes by the delete transaction that as it goes on, after a while it bogs down everything else. Splitting it up into batches limits how much data will be processed at once thus avoiding the problem and allowing pruning most or all eligible activities in the background or allow it to work at all on weaker hardware. *(Though after enough activites were cleared out by limited batches an unconstrainted query might have become feasible again and the overhead of limited btaches significant, so if you really want to clean out everything consider if it’s possible to switch to an unlimtited prune at the end)* Best reviewed commit by commit and as noted in the commit messages, many of the diff lines are just indentation adjustments and for review its probably a good idea to hide whitespace-only changes. Resolves #653 ; cc @norm
smitten reviewed 2023-12-23 22:07:23 +00:00
@ -56,0 +79,4 @@
### Options
- `--limit n` - Only delete up to `n` activities in each query. Running this task in limited batches can help maintain the instances responsiveness while still freeing up some space.
First-time contributor

I'm a little confused about if there's a difference in the behavior between this and prune_objects.

"in each query" I would understand as limiting the database lock by having smaller limit delete operations.

For prune_objects it says "limits how many remote objects get pruned initially". What does initially mean here?

I'm a little confused about if there's a difference in the behavior between this and prune_objects. "in each query" I would understand as limiting the database lock by having smaller limit delete operations. For prune_objects it says "limits how many remote objects get pruned initially". What does initially mean here?
Author
Member

"in each query" I would understand as limiting the database lock by having smaller limit delete operations.

The task executes multiple DELETE queries on the database, each of these queries will have the given limit applied. Currently it executes two queries, so running the task once wiht --limit 100 will delete at most 200 rows.
It would be possible to limit the overall deleted rows to at most exactly the given amount, but this gives preferential treatment to the first queries and since the purpose is just to limit the load and allow breaks inbetween, I figured this is not needed. But if there’s a reason to, this could be changed.

For prune_objects it says "limits how many remote objects get pruned initially". What does initially mean here?

prune_objects first deletes remote posts, then (optionally, if such flags were passed) it will run more cleanup jobs. Only the initial prune is affected by the limit the cleanup not. Reason being, that except for prune_orphaned_activities those cleanup jobs are comparatively cheap anyway.
And prune_orphaned_activities now has its own task. So if you want to cleanup some space, while not continuously hogging the db, you can first (repeatedly) run prune_objects --limit n without --prune-orphaned-activities, but all other desired cleanups in the last run. Then afterwards, repeatedly run the standalone prune_orphaned_activities --limit n as long as a single run finishes fast enough.

I pushed a new rebased version with tweaked documentation (and a typo in a commit message was fixed). Can you take a look if it’s clearer now?

> "in each query" I would understand as limiting the database lock by having smaller limit delete operations. The task executes multiple DELETE queries on the database, each of these queries will have the given limit applied. Currently it executes two queries, so running the task once wiht `--limit 100` will delete at most 200 rows. It would be possible to limit the overall deleted rows to at most exactly the given amount, but this gives preferential treatment to the first queries and since the purpose is just to limit the load and allow breaks inbetween, I figured this is not needed. But if there’s a reason to, this could be changed. > For prune_objects it says "limits how many remote objects get pruned initially". What does initially mean here? `prune_objects` first deletes remote posts, then (optionally, if such flags were passed) it will run more cleanup jobs. Only the initial prune is affected by the limit the cleanup not. Reason being, that except for `prune_orphaned_activities` those cleanup jobs are comparatively cheap anyway. And `prune_orphaned_activities` now has its own task. So if you want to cleanup some space, while not continuously hogging the db, you can first (repeatedly) run `prune_objects --limit n` *without* `--prune-orphaned-activities`, but all other desired cleanups in the last run. Then afterwards, repeatedly run the standalone `prune_orphaned_activities --limit n` as long as a single run finishes fast enough. I pushed a new rebased version with tweaked documentation (and a typo in a commit message was fixed). Can you take a look if it’s clearer now?
First-time contributor

I see what you mean, and the docs updates are clearer thanks! The steps you describe is how I was running it, I did a few prune_objects and then did a few prune_orphaned_activities.

I see what you mean, and the docs updates are clearer thanks! The steps you describe is how I was running it, I did a few `prune_objects` and then did a few `prune_orphaned_activities`.
Oneric marked this conversation as resolved
First-time contributor

This seems to be working for me! Usually pruning makes the RAM fills up on my small VPS and the instance crashes but this is running well.

This seems to be working for me! Usually pruning makes the RAM fills up on my small VPS and the instance crashes but this is running well.
Oneric force-pushed prune-batch from 80ba73839c to 3bc63afbe0 2023-12-24 23:18:28 +00:00 Compare
Oneric force-pushed prune-batch from 3bc63afbe0 to 732bc96493 2024-01-31 16:45:44 +00:00 Compare
Oneric force-pushed prune-batch from 732bc96493 to 800acfa81d 2024-02-10 01:54:09 +00:00 Compare
Author
Member

Rebased this with two updates:

  1. The logger output now shows up in stdout for me, so duplicating it with IO.puts is no longer needed iand has been dropped.
    This change also slightly confused the script from the comments; I updated it to work with the new output and made it a bit more robust wrt ordering.
  2. Standalone prune_orphaned_activities is now used in one of the orphan-pruning tests. Since both modes use the same function and the only difference is the argument parser, I figured it wasn’t worth to duplicate the test setup and instead switched one of the two orphan tests to the standalone task.

Also just because, here’s an alternative version of the script which tries to scale batch size down between some max and min value instead of immediately ceasing the prune. May be more convenient in some cases, though too low min values prob don’t make much sense (and as before time and batch sizes need tweaking for real instances).

#!/bin/sh

YIELD=120
BATCH_SIZE_MAX=200000
BATCH_SIZE_MIN=150000
BATCH_MAX_TIME=300

set -eu

# params: cur_batch_time cur_batch_size
# returns: new_batch_size (0 if constraints cannot be met; otherwise valid)
lower_batch_size() {
    # Intentional rounding imprecision to facillitate going _below_ max time
    div="$(( ($1 + BATCH_MAX_TIME - 1) / BATCH_MAX_TIME ))"
    newbatch="$(($2 / div))"
    if [ "$newbatch" -lt "$BATCH_SIZE_MIN" ] ; then
        newbatch=0
    fi
    echo "$newbatch"
}

BATCH_SIZE="$BATCH_SIZE_MAX"
echo "Starting with batch size $BATCH_SIZE"
while : ; do
    start="$(date +%s)"
    out="$( \
        mix pleroma.database prune_orphaned_activities --limit "$BATCH_SIZE" \
        | grep -E ' \[info\] Deleted ' \
    )"
    end="$(date +%s)"
	duration="$((end - start))"
	echo "$out"

	if echo "$out" | tail -n 1 | grep -qE 'Deleted 0 rows$' ; then
		echo "Nothing more to delete."
		break
	fi
	if [ "$duration" -gt "$BATCH_MAX_TIME" ] ; then
		echo "Completion of single batch takes too long ($duration > $BATCH_MAX_TIME)" >&2
        BATCH_SIZE="$(lower_batch_size "$duration" "$BATCH_SIZE")"
        if [ "$BATCH_SIZE" -gt 0 ] ; then
            echo "Try lowering batch size to $BATCH_SIZE..."
        else
    		echo "Cannot lower batch size further. Abort to not bog down instance!" >&2
		    exit 1
        fi
	fi
	sleep "$YIELD"
done
Rebased this with two updates: 1. The logger output now shows up in stdout for me, so duplicating it with `IO.puts` is no longer needed iand has been dropped. This change also slightly confused the script from the comments; I updated it to work with the new output and made it a bit more robust wrt ordering. 2. Standalone `prune_orphaned_activities` is now used in one of the orphan-pruning tests. Since both modes use the same function and the only difference is the argument parser, I figured it wasn’t worth to duplicate the test setup and instead switched one of the two orphan tests to the standalone task. Also just because, here’s an alternative version of the script which tries to scale batch size down between some max and min value instead of immediately ceasing the prune. May be more convenient in some cases, though too low min values prob don’t make much sense (and as before time and batch sizes need tweaking for real instances). ```sh #!/bin/sh YIELD=120 BATCH_SIZE_MAX=200000 BATCH_SIZE_MIN=150000 BATCH_MAX_TIME=300 set -eu # params: cur_batch_time cur_batch_size # returns: new_batch_size (0 if constraints cannot be met; otherwise valid) lower_batch_size() { # Intentional rounding imprecision to facillitate going _below_ max time div="$(( ($1 + BATCH_MAX_TIME - 1) / BATCH_MAX_TIME ))" newbatch="$(($2 / div))" if [ "$newbatch" -lt "$BATCH_SIZE_MIN" ] ; then newbatch=0 fi echo "$newbatch" } BATCH_SIZE="$BATCH_SIZE_MAX" echo "Starting with batch size $BATCH_SIZE" while : ; do start="$(date +%s)" out="$( \ mix pleroma.database prune_orphaned_activities --limit "$BATCH_SIZE" \ | grep -E ' \[info\] Deleted ' \ )" end="$(date +%s)" duration="$((end - start))" echo "$out" if echo "$out" | tail -n 1 | grep -qE 'Deleted 0 rows$' ; then echo "Nothing more to delete." break fi if [ "$duration" -gt "$BATCH_MAX_TIME" ] ; then echo "Completion of single batch takes too long ($duration > $BATCH_MAX_TIME)" >&2 BATCH_SIZE="$(lower_batch_size "$duration" "$BATCH_SIZE")" if [ "$BATCH_SIZE" -gt 0 ] ; then echo "Try lowering batch size to $BATCH_SIZE..." else echo "Cannot lower batch size further. Abort to not bog down instance!" >&2 exit 1 fi fi sleep "$YIELD" done ```
Oneric force-pushed prune-batch from afa01cb8dd to 790b552030 2024-02-19 18:36:12 +00:00 Compare
Oneric force-pushed prune-batch from 790b552030 to c127d48308 2024-05-15 01:46:32 +00:00 Compare
Author
Member

Rebased again, added some further changes and updated the initial post for the current state:

  • allow pruning array or single-object activities, separately. Only the latter need batching and checking the former also each batch just adds useless overhead
  • we only have a single activity type which can reference an array of objects, Flag, and typically there are only few of those. Using the type in the query lets it use our type index instead of scanning the entire table and probing types for every entry, speeding things up greatly
    (for single-object activities this wouldn’t help much if at all)
  • more logs documenting how pruning progresses; this makes what and if somethings happening less opaque to admins and hopefully makes long running prunes less frustrating. In any event it makes it easier to tell which part of the process got stalled and which parts are more effective
Rebased again, added some further changes and updated the initial post for the current state: - allow pruning array or single-object activities, separately. Only the latter need batching and checking the former also each batch just adds useless overhead - we only have a single activity type which can reference an array of objects, `Flag`, and typically there are only few of those. Using the type in the query lets it use our type index instead of scanning the entire table and probing types for every entry, speeding things up greatly (for single-object activities this wouldn’t help much if at all) - more logs documenting how pruning progresses; this makes what and if somethings happening less opaque to admins and hopefully makes long running prunes less frustrating. In any event it makes it easier to tell which part of the process got stalled and which parts are more effective
floatingghost reviewed 2024-05-28 01:58:17 +00:00
floatingghost left a comment
Owner

seems sensible, a few comments (they are nits)

also fails lint, so a quick format after the typo fixes would be appreciated

seems sensible, a few comments (they are nits) also fails lint, so a quick format after the typo fixes would be appreciated
@ -23,0 +43,4 @@
delete from public.activities
where id in (
select a.id from public.activities a
left join public.objects o on a.data ->> 'object' = o.data ->> 'id'

you almost certainly don't need to prefix with public - we'll be running in the akkoma db anyhow

you almost certainly don't need to prefix with `public` - we'll be running in the akkoma db anyhow
Author
Member

the public prefix already existed before and in a different context i’ve run into issues with it lacking before (update_status_visibility_counter_cache is the only trigger function which doesn't use fully qualified names with schema prefixes. Evidently this works fine during normal instance operation, but during a data-only backup restore this caused failures. Side note: this trigger is imho ridiculously complex and costly relative to the usefulness of the feature it provides (per-instance and per-visibility post count stats in admin-fe))

i can check and if it seems to work for me, remove the prefix everywhere if you want though

the `public` prefix already existed before and in a different context i’ve run into issues with it lacking before *(`update_status_visibility_counter_cache` is the only trigger function which doesn't use fully qualified names with schema prefixes. Evidently this works fine during normal instance operation, but during a data-only backup restore this caused failures. Side note: this trigger is imho ridiculously complex and costly relative to the usefulness of the feature it provides (per-instance and per-visibility post count stats in admin-fe))* i can check and if it seems to work for me, remove the prefix everywhere if you want though
@ -23,0 +56,4 @@
"""
|> Repo.query!([], timeout: :infinity)
Logger.info("Prune activity singles: deteleted #{del_single} rows...")

typo, deteleted -> deleted

typo, deteleted -> deleted
Author
Member

fixed

fixed
Oneric marked this conversation as resolved
@ -23,0 +65,4 @@
"""
delete from public.activities
where id in (
select a.id from public.activities a

same with the public prefix here

same with the public prefix here
@ -23,0 +80,4 @@
"""
|> Repo.query!([], timeout: :infinity)
Logger.info("Prune activity arrays: deteleted #{del_array} rows...")

typo here

typo here
Author
Member

fixed

fixed
Oneric marked this conversation as resolved
@ -23,0 +91,4 @@
# Flag is the only type we support with an array (and always has arrays).
# Update the only one with inlined objects, but old Update activities are
#
# We already regularly purge old Delte, Undo, Update and Remove and if

Delte -> delete

Delte -> delete
Author
Member

fixed

fixed
Oneric marked this conversation as resolved
Author
Member

One questions ahead of fixing typos and lint:

Initially (during v1) there was some confusion about if/when Logger calls get actually shown to users; since it (now) works for me and existing database tasks use only Logger i stuck with that.
However i’m still not actually sure if this reliably shows up and looking at other tasks both Logger and IO are used with the latter seemingly being more popular. On current develop (8afc3bee7a):

             Logger   IO
activity          0    2   (superfluous `require Logger`)
database          2    0
diagnostics       2    5
ecto/*            1+   0   (configures Logger level; ecto prob also use logger)
emoji             0   18
meilisearch       0   11
security          *   20  (configures Logger level)
uploads           1    0
user              0    7

refresh_counter_cache   0   0   (superfluous `require Logger`)
config uses IO.write to put config files on disk

Any opinion or guidance on what to prefer?

One questions ahead of fixing typos and lint: Initially (during v1) there was some confusion about if/when `Logger` calls get actually shown to users; since it (now) works for me and existing database tasks use only Logger i stuck with that. However i’m still not actually sure if this reliably shows up and looking at other tasks both `Logger` and `IO` are used with the latter seemingly being more popular. On current develop (8afc3bee7ae2e3119a1176d38dacb6441d8aaf3a): ``` Logger IO activity 0 2 (superfluous `require Logger`) database 2 0 diagnostics 2 5 ecto/* 1+ 0 (configures Logger level; ecto prob also use logger) emoji 0 18 meilisearch 0 11 security * 20 (configures Logger level) uploads 1 0 user 0 7 refresh_counter_cache 0 0 (superfluous `require Logger`) config uses IO.write to put config files on disk ``` Any opinion or guidance on what to prefer?

logger depends on the log level configured by the user, so if they've set :warn, it'll not show :info level logs - so for user-initiated tasks, IO is probably better

logger depends on the log level configured by the user, so if they've set :warn, it'll not show :info level logs - so for user-initiated tasks, IO is probably better
Author
Member

iirc logs shown during mix tasks didn’t seem to correlate to the regualr logger level configured for normal instance operation. But given they didn’t but now show up for me there’s clearly some setting involved (or maybe it’ſ the same setting but it somehow didn’ŧ get updated during initial recompiles, idk)

Will convert things to IO (and remove the superfluous require Logger directives)

iirc logs shown during `mix` tasks didn’t seem to correlate to the regualr logger level configured for normal instance operation. But given they didn’t but now show up for me there’s clearly some setting involved (or maybe it’ſ the same setting but it somehow didn’ŧ get updated during initial recompiles, idk) Will convert things to `IO` (and remove the superfluous `require Logger` directives)
Oneric force-pushed prune-batch from c127d48308 to 9e80ebb8d5 2024-05-30 01:40:44 +00:00 Compare
Author
Member

ok, there’s yet another way in use for putting text out: shell_info and shell_error from lib/mix/pleroma.ex. If running inside a mix shell, they output things to the mix shell, else IO.puts(msg) or IO.puts(:stderr, msg)

i guess it’s prooobably best to just use this everywhere?

ok, there’s yet another way in use for putting text out: `shell_info` and `shell_error` from `lib/mix/pleroma.ex`. If running inside a mix shell, they output things to the mix shell, else `IO.puts(msg)` or `IO.puts(:stderr, msg)` i guess it’s prooobably best to just use this everywhere?
Author
Member

hmmm, doing so breaks two test/mix/tasks/pleroma/user_test.exs tests; apparently their outputisn't caputered anymore; presumably going to the mix shell (though i didn’ŧ spot them in console output)

pushed an additional commit converting all but those two prints and some intentionally debug.-only messages in uploads to shell_* calls. Alternatively changing shell_* helpers to always use IO.puts if running in test env presumably also works, but idk where the mix.shell IO.puts distinction is relevant to begin with

hmmm, doing so breaks two `test/mix/tasks/pleroma/user_test.exs` tests; apparently their outputisn't caputered anymore; presumably going to the mix shell (though i didn’ŧ spot them in console output) pushed an additional commit converting all but those two prints and some intentionally debug.-only messages in `uploads` to `shell_*` calls. Alternatively changing `shell_*` helpers to always use `IO.puts` if running in test env presumably also works, but idk where the mix.shell IO.puts distinction is relevant to begin with
Oneric force-pushed prune-batch from 746fdd87b6 to a9d812ad7e 2024-05-30 02:35:55 +00:00 Compare
Oneric force-pushed prune-batch from a9d812ad7e to 51a7d74971 2024-05-31 02:50:08 +00:00 Compare
Author
Member

tests were easy enough to fix; everything which doesn't have a reason to use something else is now using shell_* for printing

tests were easy enough to fix; everything which doesn't have a reason to use something else is now using `shell_*` for printing
Oneric force-pushed prune-batch from 51a7d74971 to bed7ff8e89 2024-05-31 15:19:10 +00:00 Compare

everything passes, i should finally merge this

thanks a lot

everything passes, i should finally merge this thanks a lot
floatingghost merged commit 59bfdf2ca4 into develop 2024-06-17 20:47:53 +00:00
floatingghost deleted branch prune-batch 2024-06-17 20:47:53 +00:00
Sign in to join this conversation.
No description provided.