modoboa / modoboa-amavis

The amavis frontend of Modoboa
https://modoboa.org
MIT License
23 stars 14 forks source link

qcleanup fails with error #115

Open BigMichi1 opened 5 years ago

BigMichi1 commented 5 years ago

the cronjob which was installed by modoboa-installer fail every night with the following error

Cronjob:

# Quarantine cleanup
0       0       *       *       *       root    $PYTHON $INSTANCE/manage.py qcleanup

Error:

Traceback (most recent call last):
  File "/srv/modoboa/instance/manage.py", line 22, in <module>
    execute_from_command_line(sys.argv)
  File "/srv/modoboa/env/lib/python3.6/site-packages/django/core/management/__init__.py", line 364, in execute_from_command_line
    utility.execute()
  File "/srv/modoboa/env/lib/python3.6/site-packages/django/core/management/__init__.py", line 356, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/srv/modoboa/env/lib/python3.6/site-packages/django/core/management/base.py", line 283, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/srv/modoboa/env/lib/python3.6/site-packages/django/core/management/base.py", line 330, in execute
    output = self.handle(*args, **options)
  File "/srv/modoboa/env/lib/python3.6/site-packages/modoboa_amavis/management/commands/qcleanup.py", line 59, in handle
    Msgs.objects.filter(time_num__lt=limit).delete()
  File "/srv/modoboa/env/lib/python3.6/site-packages/django/db/models/query.py", line 620, in delete
    deleted, _rows_count = collector.delete()
  File "/srv/modoboa/env/lib/python3.6/site-packages/django/db/models/deletion.py", line 265, in delete
    self.data[model] = sorted(instances, key=attrgetter("pk"))
TypeError: '<' not supported between instances of 'memoryview' and 'memoryview'
tonioo commented 5 years ago

@BigMichi1 Strange, looks like a python3.6 compat issue... I'll try to reproduce it.

PatTheMav commented 5 years ago

I have the same issue and from what I can tell the root cause seems to be psycopg2 using memoryview for bytea fields under py3. Amavis‘ msgs table‘s primary key consists of an integer and that bytea, so the pk field in Django‘s model will probably be a memoryview as well. This breaks the delete function.

According to the psycopg2 devs, this is something they might fix in v3, but will stay as-is.

ironage commented 5 years ago

I observe the same error with modoboa: 1.13.1 and Amavis frontend: 1.2.3 on Ubuntu 18.04.2 LTS. The specific cron job that's failing is ./env/bin/python instance/manage.py qcleanup I'm disabling the job from /etc/cron.d/modoboa until this is fixed.

PatTheMav commented 5 years ago

@ironage Which version of Python do you run modoboa with? IIRC psycogp2 should use buffer for bytea fields under Python 2.

Django automatically orders objects by their primary keys when deleting them and as stated above the msgs table bytea field used as primary key is mapped to memoryview in Python 3 and thus not comparable.

ironage commented 5 years ago

@PatTheMav I'm using Python 3.6.7. It seems to me that memoryview should implement comparison, maybe it is an oversight. Functionally it could be the equivalent of a.tolist() < b.tolist();

PatTheMav commented 5 years ago

@ironage Well, Python 3 explicitly states: TypeError: '<' not supported between instances of 'memoryview' and 'memoryview'.

This is due to __lt__ and __gt__ not being implemented:

>>> memoryview(b"Foo").__lt__(memoryview(b"Bar"))
NotImplemented

I've dug up this post/reply by a psycopg2 developer explaining this "mess": https://www.postgresql.org/message-id/CA%2Bmi_8Zy50LYof%3DFfO9qybeQJfoLPpUXCand8CBvapS%3DXoCkGg%40mail.gmail.com

I've tried using the typecast as he suggested but first I had to adapt to my specific CFFI case (PyPy FTW) by encoding the value to a byte string (just add those lines to the import statements in settings.py):

try:
    import psycopg2
except ImportError:
    # Fall back to psycopg2cffi
    from psycopg2cffi import compat
    compat.register()

    import psycopg2

def bytea2bytes(value, cur):
    if value is None:
        return None
    buf = psycopg2.BINARY(value.encode('utf-8'), cur)

    if buf is not None:
        return buf.tobytes()

BYTEA2BYTES = psycopg2.extensions.new_type(
    psycopg2.BINARY.values, 'BYTEA2BYTES', bytea2bytes
)

psycopg2.extensions.register_type(BYTEA2BYTES)

YMMV, but with PyPy 3.6 7.0.0 this works without throwing any errors now.

ironage commented 5 years ago

@PatTheMav many thanks for that workaround! I applied the patch and it's working for me as well. I'll enable the cron job again. Probably the issue is really the fault of psycopg2 for not using a type that supports sorting by default, but I also think the memoryview should support comparison in this case. I found an open issue about it and left a comment there providing this issue as an "in the wild" argument for supporting it (https://bugs.python.org/issue20399).

proto commented 5 years ago

Many thanks, it's working for me too.

Sorry @tonioo for duplicate.

tonioo commented 4 years ago

@PatTheMav Where did you put your fix in the code?

PatTheMav commented 4 years ago

@tonioo I updated settings.py and put this right below import os:

try:
    import psycopg2
except ImportError:
    # Fall back to psycopg2cffi
    from psycopg2cffi import compat
    compat.register()

    import psycopg2

def bytea2bytes(value, cur):
    if value is None:
        return None
    buf = psycopg2.BINARY(value.encode('utf-8'), cur)

    if buf is not None:
        return buf.tobytes()

BYTEA2BYTES = psycopg2.extensions.new_type(
    psycopg2.BINARY.values, 'BYTEA2BYTES', bytea2bytes
)

psycopg2.extensions.register_type(BYTEA2BYTES)

I also had to add a single psycopg2.py in the site-packages directory with the following content to make it work:

from psycopg2cffi import compat
compat.register()

Alas I don't know which module needed this to work or if this was some compatibility fix with Django, I will have to investigate this again. But this is psycopg2cffi specific for Pypy3 support.

EDIT: It should be noted that I have modoboa with (I think) all plugins working on Pypy3 with psycopg2 and gunicorn with eventlet workers for over a year now without issues.

It's not covered by the installer, but I set up my modoboa server twice now without it and.. I learned a lot from that.. 😂

pubmania commented 4 years ago

@PatTheMav Thanks for this. I have applied the patch and I hope it will work. For the psycopg2.py file that is to be created in site-packages... am I correct to place it at following path:

/srv/modoboa/env/lib/python3.6/site-packages

PatTheMav commented 4 years ago

@pubmania Yeah straight in there, but I'd advise leaving it for now as I can't recall what I needed that fix for and it's more or less a hack that should be avoided if necessary.

My gut tells me that was a specific "hack" for psycopg2cffi and if you don't use PyPy it's not needed.

pubmania commented 4 years ago

Cheers 👍👍

I created the file so might as well leave it there if it won't harm anything or do you reckon deleting is better way to go.

PatTheMav commented 4 years ago

Well on second thought it has to be a workaround for pypy as psycopg2cffi is used specifically for that purpose, it doesn't really make sense to use it with CPython (as psycopg2 has more recent updates and there is no performance penalty for C modules), so that dummy file just covers modules that load/require a psycopg2 file for an import that would otherwise fail.

So bottom line: If you use CPython, you only need to place this in the file if you actually use a Postgres database.

import psycopg2

def bytea2bytes(value, cur):
    if value is None:
        return None
    buf = psycopg2.BINARY(value.encode('utf-8'), cur)

    if buf is not None:
        return buf.tobytes()

BYTEA2BYTES = psycopg2.extensions.new_type(
    psycopg2.BINARY.values, 'BYTEA2BYTES', bytea2bytes
)

psycopg2.extensions.register_type(BYTEA2BYTES)
pubmania commented 4 years ago

ah I don't know much about CPython so it is safe to assume I don't use it but I do use Postgres database. Thanks for taking the time to explain.

PatTheMav commented 4 years ago

CPython is the name of the reference implementation of Python, so the one you usually download from their website (and that apt installs), so that's probably the one you have in use. 😉

pubmania commented 4 years ago

lol... damn so I do use it

alisonw commented 4 years ago

Just to throw in I got this message ("TypeError: '<' not supported between instances of 'memoryview' and 'memoryview'") for the first time overnight, so the issue is still around even with fully up-to-date Modoboa installation. The box seems to still be working on through though so I haven't (yet, anyway) tried the patch above.

Full Traceback (most recent call last):

  File "/srv/modoboa/instance/manage.py", line 21, in <module>
    main()
  File "/srv/modoboa/instance/manage.py", line 17, in main
    execute_from_command_line(sys.argv)
  File "/srv/modoboa/env/lib/python3.6/site-packages/django/core/management/__init__.py", line 381, in execute_from_command_line
    utility.execute()
  File "/srv/modoboa/env/lib/python3.6/site-packages/django/core/management/__init__.py", line 375, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/srv/modoboa/env/lib/python3.6/site-packages/django/core/management/base.py", line 323, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/srv/modoboa/env/lib/python3.6/site-packages/django/core/management/base.py", line 364, in execute
    output = self.handle(*args, **options)
  File "/srv/modoboa/env/lib/python3.6/site-packages/modoboa_amavis/management/commands/qcleanup.py", line 59, in handle
    Msgs.objects.filter(time_num__lt=limit).delete()
  File "/srv/modoboa/env/lib/python3.6/site-packages/django/db/models/query.py", line 711, in delete
    deleted, _rows_count = collector.delete()
  File "/srv/modoboa/env/lib/python3.6/site-packages/django/db/models/deletion.py", line 266, in delete
    self.data[model] = sorted(instances, key=attrgetter("pk"))
TypeError: '<' not supported between instances of 'memoryview' and 'memoryview'
FranMercedesG commented 4 years ago

Well on second thought it has to be a workaround for pypy as psycopg2cffi is used specifically for that purpose, it doesn't really make sense to use it with CPython (as psycopg2 has more recent updates and there is no performance penalty for C modules), so that dummy file just covers modules that load/require a psycopg2 file for an import that would otherwise fail.

So bottom line: If you use CPython, you only need to place this in the file if you actually use a Postgres database.

import psycopg2

def bytea2bytes(value, cur):
    if value is None:
        return None
    buf = psycopg2.BINARY(value.encode('utf-8'), cur)

    if buf is not None:
        return buf.tobytes()

BYTEA2BYTES = psycopg2.extensions.new_type(
    psycopg2.BINARY.values, 'BYTEA2BYTES', bytea2bytes
)

psycopg2.extensions.register_type(BYTEA2BYTES)

Where I should put that?

on /srv/mobodoa/instance/instance/settings.py ?

PatTheMav commented 4 years ago

Yep, as I mentioned in https://github.com/modoboa/modoboa-amavis/issues/115#issuecomment-630104853.

FranMercedesG commented 4 years ago

Ok, I no longer have errors, but it is not cleaning the emails that are in quarantine

kisst commented 3 years ago

This issue still exist on python3.7 as well.

  File "/srv/modoboa/env/lib/python3.7/site-packages/django/db/models/deletion.py", line 266, in delete
    self.data[model] = sorted(instances, key=attrgetter("pk"))
TypeError: '<' not supported between instances of 'memoryview' and 'memoryview'
PatTheMav commented 3 years ago

It will continue to exist for as long as you use a Postgres database and Django uses psycopg2.

psycopg3 is in development, but far from production ready and all signs point to this never being fixed, because it only happens in specific circumstances, mainly using a composite primary key for a Django model which is made up of a scalar and a bytea field. Django does some automatic sorting and this is where it will trigger an (impossible) comparison.

The actual fix would be for Amavis(!) to not use a composite primary key with a bytea field or Django to detect this and be smarter in its models.

ksaadDE commented 3 years ago

Hello there, what does the code exactly? Since this information provides the possibility to improve the code, so it will not throw errors in the future I didn't looked at it directly, but I got a lot of these exceptions thrown and I see in the conversation here, that the code does something with the amavis modoboa plugin / spam quarantine "filter"

psycopg3 is in development, but far from production ready so then why it's mentioned (or even implemented) here?

due to: Django to detect this and be smarter in its models. As far as I know is the Django Community a bit bigger, maybe we should ask them if they could implement it?

stefaweb commented 1 year ago

Hi Folk!

I was out for 3 weeks.

I have the same problem for 2 weeks now with last modoboa version.

I have this version:

/srv/modoboa/env/lib/python3.9/site-packages/psycopg2

Traceback (most recent call last):
  File "/srv/modoboa/instance/manage.py", line 22, in <module>
    main()
  File "/srv/modoboa/instance/manage.py", line 18, in main
    execute_from_command_line(sys.argv)
  File "/srv/modoboa/env/lib/python3.9/site-packages/django/core/management/__init__.py", line 419, in execute_from_command_line
    utility.execute()
  File "/srv/modoboa/env/lib/python3.9/site-packages/django/core/management/__init__.py", line 413, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/srv/modoboa/env/lib/python3.9/site-packages/django/core/management/base.py", line 354, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/srv/modoboa/env/lib/python3.9/site-packages/django/core/management/base.py", line 398, in execute
    output = self.handle(*args, **options)
  File "/srv/modoboa/env/lib/python3.9/site-packages/modoboa_amavis/management/commands/qcleanup.py", line 65, in handle
    qset.delete()
  File "/srv/modoboa/env/lib/python3.9/site-packages/django/db/models/query.py", line 746, in delete
    deleted, _rows_count = collector.delete()
  File "/srv/modoboa/env/lib/python3.9/site-packages/django/db/models/deletion.py", line 382, in delete
    self.data[model] = sorted(instances, key=attrgetter("pk"))
TypeError: '<' not supported between instances of 'memoryview' and 'memoryview'