python plugins: enable direct I/O in core instead of calling python for read and write #1297

pstorz · 2022-10-28T08:13:33Z

The filedaemon api calls the plugin for doing the actual file I/O (i.e. reading during backup and writing during restore).
This is by far the most frequently called function in the plugin api and thus can have big impact on the overall performance.

This PR enables python plugins to pass the filedescriptor that is intended to do the I/O in the open call of the file in
plugin_io_open(), IOP.filedes needs to be set to the filedescriptor for the file, and IOP.status needs to be set to bareosfd.iostat_do_in_core. Otherwise the IOP.status should be set to bareosfd.iostat_do_in_plugin
Example:

            # do io in core
            IOP.filedes = self.file.fileno()
            IOP.status =  bareosfd.iostat_do_in_core
 
            #  do io in plugin
            #IOP.status = bareosfd.iostat_do_in_plugin

In our test environment, the backup speed could be more than doubled so that the backup via python plugin was as fast as a backup directly via the filedaemon.

Please check

Short description and the purpose of this PR is present above this paragraph
Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

General

PR name is meaningful
Purpose of the PR is understood
Commit descriptions are understandable and well formatted
Check backport line
Is the PR title usable as CHANGELOG entry?
Separate commit for CHANGELOG.md ("update CHANGELOG.md"). The PR number is correct.

Source code quality

Source code changes are understandable
Variable and function names are meaningful
Code comments are correct (logically and spelling)
Required documentation changes are present and part of the PR
bareos-check-sources --since-merge does not report any problems

Tests

Decision taken that a test is required (if not, then remove this paragraph)
The choice of the type of test (unit test or systemtest) is reasonable
Testname matches exactly what is being tested
On a fail, output of the test leads quickly to the origin of the fault

arogge

I think we should try to get rid of do_io_in_core and handle that with magic values for filedes.

core/src/filed/fd_plugins.cc

core/src/filed/fd_plugins.h

core/src/filed/fd_plugins.cc

core/src/filed/fd_plugins.h

arogge

Looks a lot simpler already. Still some caveats, but steadily improving.

core/src/filed/fd_plugins.cc

arogge · 2022-11-23T16:14:37Z

core/src/filed/fd_plugins.cc

+  //      - Set the io.filedes to the filedescriptor of the file that was
+  //        opened in the plugin. In this case the core code will read from
+  //        write to that filedescriptor during backup and restore.
+  //  2.: - Set io.status to IoStatus::success, and


was this truncated? At least it doesn't make sense to me.

Yes, this was updated.

no, still truncated :(

core/src/plugins/filed/python/module/bareosfd.h

docs/manuals/source/DeveloperGuide/PythonPluginAPI.rst

core/src/plugins/filed/python/pyfiles/BareosFdPluginLocalFileset.py

arogge

Some minor scoping and re-initialization issues left.
Also, we should probably change the struct io_pkt everywhere throughout the codebase.

core/src/filed/fd_plugins.cc

arogge · 2022-11-30T13:00:28Z

core/src/filed/fd_plugins.cc

+  //      - Set the io.filedes to the filedescriptor of the file that was
+  //        opened in the plugin. In this case the core code will read from
+  //        write to that filedescriptor during backup and restore.
+  //  2.: - Set io.status to IoStatus::success, and


no, still truncated :(

core/src/plugins/filed/python/test/python-fd-module-tester.cc

core/src/filed/fd_plugins.h

core/src/filed/fd_plugins.cc

arogge

some really minor things left. Sorry.

core/src/plugins/filed/python/module/bareosfd.cc

core/src/plugins/filed/python/module/bareosfd.h

systemtests/tests/py2plug-fd-local-fileset-basic/test-cleanup

arogge

bareos-check-source brought up some issues and the test configuration probably doesn't work as intended.

core/src/plugins/filed/python/test/bareosfd_test.py

systemtests/tests/py2plug-fd-local-fileset-basic/test-cleanup

systemtests/tests/py2plug-fd-local-fileset-basic/test-setup

systemtests/tests/py2plug-fd-local-fileset-basic/CMakeLists.txt

Implement `plugin_io_open()` so that we can configure direct I/O and pass the filedescriptor.

Sentinels for PyRestorePacket_members and PyIoPacket_members were incomplete and are now corrected.

Change every wrong use of "#" to comment out a bconsole command to the correct "@#" that will actually comment out the command and will not produce the error message "#: is an invalid command."

As do_io_in_core was removed from IoPacket again, the test should not check for it.

In bareosJobMsg() and bareosDebugMsg()

Change python-local-fileset-test to also use io_in_core.

The basic and io_in_core tests in py{2,3}plug-fd-local-fileset-basic will run into UTF-8 conversion errors when running under the python3 plugin. Until this problem is fixed, these tests are now marked as broken.

enum IoStatus: wrap into namespace

Replaces exec_program() with the newer execute_process().

The tests in pyplug-fd-local-fileset-basic are now enabled and disabled as originally intended.

Change status messages to use unicode "check mark" and "ballot x" when showing if a test was enabled/disabled. Also prints the directory containing the test now.

Changes the implementation of BareosFdPluginLocalFilesBaseclass to handle non-utf8 encodable file-names gracefully. Instead of crashing we will now emit an M_ERROR that the file's name cannot be represented and go on with the next file.

due to issues with multibyte files in the baseclass when using python3, we mark this test as broken for python3, as it will fail when python thinks the filesystem is in ascii-mode.

pstorz force-pushed the dev/pstorz/master/plugin_io_in_core branch from 5f4651c to 99c4c04 Compare October 28, 2022 11:12

pstorz requested a review from arogge October 28, 2022 15:25

bruno-at-bareos assigned pstorz Nov 3, 2022

pstorz force-pushed the dev/pstorz/master/plugin_io_in_core branch from 3f3fe3a to 43b89d7 Compare November 3, 2022 16:51

arogge requested changes Nov 7, 2022

View reviewed changes

pstorz force-pushed the dev/pstorz/master/plugin_io_in_core branch from d23856d to 6910cdc Compare November 11, 2022 19:29

bruno-at-bareos requested a review from arogge November 17, 2022 11:04

arogge reviewed Nov 17, 2022

View reviewed changes

core/src/filed/fd_plugins.h Outdated Show resolved Hide resolved

arogge requested changes Nov 23, 2022

View reviewed changes

pstorz force-pushed the dev/pstorz/master/plugin_io_in_core branch 2 times, most recently from 82b0b0a to 847d7cb Compare November 29, 2022 17:48

pstorz requested a review from arogge November 30, 2022 11:48

arogge requested changes Nov 30, 2022

View reviewed changes

pstorz added the Release critical 22 label Dec 1, 2022

pstorz force-pushed the dev/pstorz/master/plugin_io_in_core branch from 84c66e1 to aa4534d Compare December 1, 2022 11:50

pstorz requested a review from arogge December 1, 2022 12:21

arogge requested changes Dec 1, 2022

View reviewed changes

core/src/plugins/filed/python/module/bareosfd.cc Outdated Show resolved Hide resolved

core/src/plugins/filed/python/module/bareosfd.h Outdated Show resolved Hide resolved

systemtests/tests/py2plug-fd-local-fileset-basic/test-cleanup Show resolved Hide resolved

pstorz requested a review from arogge December 5, 2022 08:20

arogge approved these changes Dec 5, 2022

View reviewed changes

arogge requested changes Dec 5, 2022

View reviewed changes

pstorz force-pushed the dev/pstorz/master/plugin_io_in_core branch from 1e2fb47 to 9952746 Compare December 5, 2022 13:24

arogge force-pushed the dev/pstorz/master/plugin_io_in_core branch 3 times, most recently from 6cec9e2 to 74eb304 Compare December 5, 2022 18:22

arogge approved these changes Dec 5, 2022

View reviewed changes

arogge enabled auto-merge December 5, 2022 18:29

arogge force-pushed the dev/pstorz/master/plugin_io_in_core branch from 2800b28 to 3bb485a Compare December 6, 2022 14:56

pstorz added 3 commits December 7, 2022 13:18

rename BareosWinFilePacket -> BareosFilePacket

f85bae2

BareosFilePacket: rename fid -> filedes

f6bf7c4

BareosFilePacket: add bool do_io_in_core

83497b0

pstorz and others added 25 commits December 7, 2022 13:18

BareosFdPluginLocalFileset.py: use direct I/O

73e934a

Implement `plugin_io_open()` so that we can configure direct I/O and pass the filedescriptor.

PythonPluginAPI.rst: add text formatting

200613f

plugins: fix sentinels in bareosfd.h

da33c5a

Sentinels for PyRestorePacket_members and PyIoPacket_members were incomplete and are now corrected.

removed do_io_in_core, now works via IOP.status

ebebb26

systemtests: fix bconsole comments

2ff3b47

Change every wrong use of "#" to comment out a bconsole command to the correct "@#" that will actually comment out the command and will not produce the error message "#: is an invalid command."

pyplug-local-fileset-restoreobject: debug client

e94a4b4

bareosfd_test.py: adapt to changed IoPacket type

ad89f23

As do_io_in_core was removed from IoPacket again, the test should not check for it.

python-fd-module-tester.cc: use varargs

3f97870

In bareosJobMsg() and bareosDebugMsg()

bareosfd.h: rename python variables

de718f2

systemtests: adapt python-local-fileset-test

400380c

Change python-local-fileset-test to also use io_in_core.

python tests: adapt bareosfd test to iostat names

2395df4

systemtests: label tests with UTF-8 issues broken

b016b75

The basic and io_in_core tests in py{2,3}plug-fd-local-fileset-basic will run into UTF-8 conversion errors when running under the python3 plugin. Until this problem is fixed, these tests are now marked as broken.

plugin io in core: update PythonPluginAPI docs

0fd6ea2

bareosfd.h: remove do_io_in_core from PyIoPacket

d0d8b37

fd_plugins.cc: fix comment

3170bd3

struct io_pkt: initialize instead of memset()

3e43958

other structs: initialize instead of memset()

18633e5

introduced kInvalidFiledescriptor

5416e9e

fd_plugins.cc: add review suggested changes

01181a3

enum IoStatus: wrap into namespace

python-fd-module-tester: use vsnprintf

f21992c

plugins: remove struct keyword for all *_pkt types

a46560a

BareosCheckAcl.cmake: use execute_process

fbd0767

Replaces exec_program() with the newer execute_process().

systemtests: correctly enable local-filset-basic

ca89b6b

The tests in pyplug-fd-local-fileset-basic are now enabled and disabled as originally intended.

systemtests: improve cmake output for adding tests

3d11527

Change status messages to use unicode "check mark" and "ballot x" when showing if a test was enabled/disabled. Also prints the directory containing the test now.

arogge force-pushed the dev/pstorz/master/plugin_io_in_core branch from da73322 to 9ffa0bf Compare December 7, 2022 12:38

systemtests: set io_in_core py3 test broken

b39cdeb

due to issues with multibyte files in the baseclass when using python3, we mark this test as broken for python3, as it will fail when python thinks the filesystem is in ascii-mode.

arogge force-pushed the dev/pstorz/master/plugin_io_in_core branch from 9ffa0bf to b39cdeb Compare December 7, 2022 12:53

arogge merged commit 23d60b0 into master Dec 7, 2022

pstorz deleted the dev/pstorz/master/plugin_io_in_core branch April 17, 2023 14:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

python plugins: enable direct I/O in core instead of calling python for read and write #1297

python plugins: enable direct I/O in core instead of calling python for read and write #1297

pstorz commented Oct 28, 2022 •

edited by arogge

arogge left a comment

arogge left a comment

arogge Nov 23, 2022

pstorz Nov 29, 2022

arogge Nov 30, 2022

arogge left a comment

arogge Nov 30, 2022

arogge left a comment

arogge left a comment

python plugins: enable direct I/O in core instead of calling python for read and write #1297

python plugins: enable direct I/O in core instead of calling python for read and write #1297

Conversation

pstorz commented Oct 28, 2022 • edited by arogge

Please check

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

General

Source code quality

Tests

arogge left a comment

Choose a reason for hiding this comment

arogge left a comment

Choose a reason for hiding this comment

arogge Nov 23, 2022

Choose a reason for hiding this comment

pstorz Nov 29, 2022

Choose a reason for hiding this comment

arogge Nov 30, 2022

Choose a reason for hiding this comment

arogge left a comment

Choose a reason for hiding this comment

arogge Nov 30, 2022

Choose a reason for hiding this comment

arogge left a comment

Choose a reason for hiding this comment

arogge left a comment

Choose a reason for hiding this comment

pstorz commented Oct 28, 2022 •

edited by arogge