unoconv conversion fails from within trytond

added 1 deleted label

We found unoconv to not be thread safe. We are running with a listener and the patch at
https://gitlab.com/m9s/trytond-patches/blob/mbs-3.4/unoconv_retry.patch
without problems (usually we set retry to 5 in trytond.conf). Perhaps it can you help, too.

added 1 deleted label and removed 1 deleted label

@yangoon why you don't propose the patch to be included in trytond?

> Sergi Almacellas Abellana <sergi@koolpi.com> added the comment:
>
> @yangoon why you don't propose the patch to be included in trytond?

IIRC we did that yet for 2.2 and at that time it was not accepted.

> Mathias Behrle <mbehrle@m9s.biz> added the comment:
>
>> Sergi Almacellas Abellana <sergi@koolpi.com> added the comment:
>>
>> @yangoon why you don't propose the patch to be included in trytond?
>
> IIRC we did that yet for 2.2 and at that time it was not accepted.

I haven't found any issue on the bugtracker, could you please give some link so I can follup up? Thanks

I can confirm this bug on tryton 4.0, when PDF format is setted

On 04/10/16 18:19, Oscar Andres Alvarez wrote:
> Oscar Andres Alvarez <oscar.alvarez.montero@gmail.com> added the comment:
>
> I can confirm this bug on tryton 4.0, when PDF format is setted
>
> ----------
> nosy: +oscar
>
> _______________________________________________
> Tryton issue tracker @tryton.org>
> <https://bugs.tryton.org/#5907>
> _______________________________________________
Thanks, Oscar. A major inconvenience, a I do not want to install
LibreOffice locally on the workstation.
Imagine Africa, brittle infrastructure, poor access to reliable mains
power, patchy internet cover and enthusiasitc, but poorly trained users:
Less is more. PDF wins.

Chris

Why not removing unoconv and instead just use the command line option of LibeOffice:

soffice --headless --convert-to pdf file.odt

Of course we will have to start a new process for each conversion but it is already the case with the default option.
But the main advantage is that it does not rely on the uno/pyuno API of LibreOffice which shows to be not very reliable.

Here is review34341002 that implements it.
It will be great to have feedback from different setup with different kind of loads (concurrent users etc.)

assigned to @ced

added trytond + 1 deleted label and removed 1 deleted label

New review34341002 at https://codereview.tryton.org/34341002/#ps20001

Someone has tried to use unoconv as server?

The issue with the current implementation is that any call to unoconv starts a new server to make the conversion. This creates conflicts on the instance of oo.

We fixed that by:
- launching unoconv as a standalone server (--listener)
- change trytond calls to be client only (--no-launch)
Since that change, unoconv is much more stable

NB. The issue with that review is that no more server is possible and we are not sure to get conflicts gone since they are due to libreoffice.

On 2016-10-25 17:44, Ali Kefia wrote:
> - launching unoconv as a standalone server (--listener)
> - change trytond calls to be client only (--no-launch)
> Since that change, unoconv is much more stable

Still possible by provide a custom soffice script in the PATH.

> NB. The issue with that review is that no more server is possible and we are not sure to get conflicts gone since they are due to libreoffice.

This should be tested.

I have tested three loops running at the same time converting 100 odt each one.
I have seen no conflicts so far and I see three soffice process running at the same time.

I made some tests and there are lost calls

(test the difference with and without &)

#!/bin/bash

for i in $(seq 1 10)
do
( echo "$i" && soffice --headless --nolockcheck --nodefault --norestore --convert-to pdf --outdir ./out "./odt/test-$i.odt" &> /dev/null && echo "$i => $?" ) &
done

What do you mean by "lost calls"?

I tested with soffice 5.1.4.2 and I got all the pdf generated.
Of course it is a little bit slow on my small machine but it works.

I think you should look at the error output to understand what is happening on your side.

Nothing on strerr nor on stdout
LibreOffice 4.2.8.2 420m0(Build:2)

Version 4.2.8.2 is 2 years old, it is considered by LibreOffice as obsolete.
You should try with a more up to date version.

The problem is that version 4.x is the default on Debian (a usual distribution for deployment)
https://packages.debian.org/jessie/libreoffice

Tried on local and on a newer Office version via Docker, the result is the same (I have an image on my test.odt document)

Scripts are here: https://gist.github.com/alikefia/32b659deb96bc5d1eb4726ce0727177e

## Local:

~/c/s/pdf ❯❯❯ ./convert 10
converting 10 files
LibreOffice 4.2.8.2 420m0(Build:2)

8 => 1
4 => 1
6 => 0
2 => 0
3 => 0
1 => 0
9 => 0
7 => 0
5 => 0
10 => 0
8 pdf generated

## Newer version:

~/c/s/pdf ❯❯❯ docker run -it pdf 10
converting 10 files
LibreOffice 5.1.4.2 9b41a63f5af9c5b93dae98ccbfcda2d4ec09ef7a

6 => 1
9 => 77
5 => 0
4 => 0
10 => 0
3 => 0
2 => 0
1 => 0
8 => 0
7 => 0
8 pdf generated

I guess your script is not correctly waiting all the jobs.
soffice is not the executable.

It should continue writing since jobs are launched as an async subshell

Have pushed a kind of bench, if someone is interested to test:
https://github.com/coopengo/trytond-report-bench

Maybe but I still do not buy your test script. If you look at the start-up script of soffice, it does many exec and one is oosplash.
Indeed you should wait for each jobs.

does oo script work on your machine?
PS: even if master script leaves, execution continues

On 2016-10-26 16:26, Ali Kefia wrote:
> does oo script work on your machine?

No because it is bashism.

> PS: even if master script leaves, execution continues

But you still count the wrong numbers.

I think we should allow to override the way how the conversion is to be done. with the planned override mechanism for base classes this should be doable easily. I think it would be good to move the conversion out to some workers. Especially in bigger installations or installations with bigger documents this could be a good solution. For the future i think it would be good to have another report engine that focuses on creating pdfs for all reports that do not need to be convertable.

See this tool of Coopengo, maybe useful for test concurrency reports
https://github.com/coopengo/trytond-report-bench

On 2016-10-26 17:35, Korbinian Preisler wrote:
> I think we should allow to override the way how the conversion is to be done. with the planned override mechanism for base classes this should be doable easily.

Yes of course this will be a good improvement and by the way the
refactoring of Report from 3.6 defines a single method for format
conversion.

> I think it would be good to move the conversion out to some workers. Especially in bigger installations or installations with bigger documents this could be a good solution.

This will be done like for the redis cache. But by default, we need a
simple solution that works out of the box.

For what I understand, the failure are due to missing resources (maybe memory as we start a many processes).
So I will update the patch to make few retries in such case but I will also delay it until we have the mechanism to extend globally the Report class.

added 1 deleted label and removed 1 deleted label

Indeed I will not add a loop because for now the patch will raise an issue in such case and also if there are not enough resources relaunching the process will not help.

>Someone has tried to use unoconv as server?

>The issue with the current implementation is that any call to unoconv starts a new server to make the conversion. This creates conflicts on the instance of oo.

>We fixed that by:
>- launching unoconv as a standalone server (--listener)
>- change trytond calls to be client only (--no-launch)
>Since that change, unoconv is much more stable

>NB. The issue with that review is that no more server is possible and we are not sure to get conflicts gone since they are due to libreoffice.

I tested similar solution, but I still got errors sometimes like it is explained here: https://groups.google.com/d/msg/tryton/prztgxYjm0I/dqCLOPulCAAJ

Similar error like explained in msg28993.

review34341002 updated at https://codereview.tryton.org/34341002/#ps40001

review34341002 updated at https://codereview.tryton.org/34341002/#ps80001

review34341002 updated at https://codereview.tryton.org/34341002/#ps100001

I tried this latest review patch (34341002) adjusted for 4.6
and it seems to work okay.

+1

review34341002 updated at https://codereview.tryton.org/34341002/#ps120001

Indeed we do not need the #5032 (closed) but only #4735 (closed) to make Report.convert customizable.

added 1 deleted label and removed 1 deleted label

New changeset 803001d43432 by Cédric Krier in branch 'default':
Replace unoconv by calling soffice directly
http://hg.tryton.org/trytond/rev/803001d43432

added 1 deleted label and removed 1 deleted label

closed

mentioned in issue #5914 (closed)

mentioned in issue #6025 (closed)

mentioned in issue #6929 (closed)

unoconv conversion fails from within trytond

Child items 0

Activity

Admin message

unoconv conversion fails from within trytond

Activity