Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

devel: Speed up leapp database writes by disabling synchronization #732

Merged
merged 1 commit into from
Sep 21, 2021

Conversation

vinzenz
Copy link
Member

@vinzenz vinzenz commented Sep 21, 2021

This patch introduces an optimization of the database access when the
LEAPP_DEVEL_DATABASE_SYNC_OFF environment variable is set.

This causes to write to the database up to 20 times faster than normal,
depending on the underlying hard drive and file system.

I managed to increase the speed leapp preupgrade from 9 minutes 7
seconds to 26 seconds.

However this feature comes at the cost of risking database corruption,
in case of powerloss or OS crashes. According to the SQLite
documentation, application crashes are safe.

Since this option poses a potential risk, it is deemed for now
a development option only and will help to speed up our CI systems.

@github-actions
Copy link

Thank you for contributing to the Leapp project!

Please note that every PR needs to comply with the Leapp Guidelines and must pass all tests in order to be mergable.
If you want to re-run tests or request review, you can use following commands as a comment:

  • leapp-ci build to run copr build and e2e tests in OAMG CI
  • review please to notify leapp developers of review request
  • /packit copr-build to submit a public copr build using packit

Please open ticket in case you experience technical problem with the CI. (RH internal only)

Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please consider rerunning the CI by commenting leapp-ci build (might require several comments). If the problem persists, contact leapp-infra.

This patch introduces an optimization of the database access when the
LEAPP_DEVEL_DATABASE_SYNC_OFF environment variable is set.

This causes to write to the database up to 20 times faster than normal,
depending on the underlying hard drive and file system.

I managed to increase the speed leapp preupgrade from 9 minutes 7
seconds to 26 seconds.

However this feature comes at the cost of risking database corruption,
in case of powerloss or OS crashes. According to the SQLite
documentation, application crashes are safe.

Since this option poses a potential risk, it is deemed for now
a development option only and will help to speed up our CI systems.
Copy link
Member

@fernflower fernflower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it!

@fernflower
Copy link
Member

/rerun

@github-actions
Copy link

Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/2831780

@github-actions
Copy link

Testing Farm request for tmt test was created. Once finished, results should be available here.
Full pipeline log.

Copy link
Member

@pirat89 pirat89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love it. and so far, it still works. as it is covered by a "LEAPP_DEVEL_*" envar, it's safe to merge it. We will document additional envars soon, so merging it even when it's still not documented.

@pirat89 pirat89 merged commit 25ab116 into oamg:master Sep 21, 2021
@fernflower
Copy link
Member

Tested on vms from 1MT - 40-50% speedup.
Default no env var case:

real	23m17.276s
user	7m13.111s
sys	1m57.130s

Env var set:

real	12m45.962s
user	6m59.196s
sys	1m49.907s

@vinzenz vinzenz deleted the devel-speedup-leapp branch September 22, 2021 07:11
pirat89 added a commit to pirat89/leapp that referenced this pull request Oct 19, 2021
## Packaging
- Drop the dependency on leapp-repository for Fedora and RHEL 8+ (oamg#717)
- Provide builds for RHEL 7+ and Fedora (oamg#717)
- Drop automatically generated Python dependences on RHEL 8+ and Fedora systems (oamg#717, oamg#716)
- Bump the provided leapp-framework capability to 2.0 (oamg#700)

## Framework
### Fixes
- models: Do not make references to private symbols (oamg#718)

### Enhancements
- Introduce the LEAPP_DEVEL_DATABASE_SYNC_OFF envar to enable speed up writes into the leapp database by disabling synchronisation - only for development / testing purposes (oamg#732)

## Leapp
### Fixes
- Fix print of the leapp help msg for Python 3.6+ (oamg#731)

### Enhancements
- The leapp commands are now defined/provided by leapp-repositories; leapp discovers the specified commands automatically (oamg#700)
- Add CLI support for `choices` and `default` for options of leapp commands (oamg#734)

## Modifications
- Makefile: Added the `fast_lint` target to apply linters just on files different from master (oamg#733)
@pirat89 pirat89 mentioned this pull request Oct 19, 2021
pirat89 added a commit to pirat89/leapp that referenced this pull request Oct 19, 2021
## Packaging
- Drop the dependency on leapp-repository for Fedora and RHEL 8+ (oamg#717)
- Provide builds for RHEL 7+ and Fedora (oamg#717)
- Drop automatically generated Python dependences on RHEL 8+ and Fedora systems (oamg#717, oamg#716)
- Bump the provided leapp-framework capability to 2.0 (oamg#700)

## Framework
### Fixes
- models: Do not make references to private symbols (oamg#718)

### Enhancements
- Introduce the LEAPP_DEVEL_DATABASE_SYNC_OFF envar to enable speed up writes into the leapp database by disabling synchronisation - only for development / testing purposes (oamg#732)

## Leapp
### Fixes
- Fix print of the leapp help msg for Python 3.6+ (oamg#731)

### Enhancements
- The leapp commands are now defined/provided by leapp-repositories; leapp discovers the specified commands automatically (oamg#700)
- Add CLI support for `choices` and `default` for options of leapp commands (oamg#734)

## Modifications
- Makefile: Added the `fast_lint` target to apply linters just on files different from master (oamg#733)
pirat89 added a commit that referenced this pull request Oct 19, 2021
## Packaging
- Drop the dependency on leapp-repository for Fedora and RHEL 8+ (#717)
- Provide builds for RHEL 7+ and Fedora (#717)
- Drop automatically generated Python dependences on RHEL 8+ and Fedora systems (#717, #716)
- Bump the provided leapp-framework capability to 2.0 (#700)

## Framework
### Fixes
- models: Do not make references to private symbols (#718)

### Enhancements
- Introduce the LEAPP_DEVEL_DATABASE_SYNC_OFF envar to enable speed up writes into the leapp database by disabling synchronisation - only for development / testing purposes (#732)

## Leapp
### Fixes
- Fix print of the leapp help msg for Python 3.6+ (#731)

### Enhancements
- The leapp commands are now defined/provided by leapp-repositories; leapp discovers the specified commands automatically (#700)
- Add CLI support for `choices` and `default` for options of leapp commands (#734)

## Modifications
- Makefile: Added the `fast_lint` target to apply linters just on files different from master (#733)
@xmontagut
Copy link

You saved my day, TY. Without this improvment, after 15 min, the leapp preupgrade was still at the PCI scan step...

@anandanramadoss
Copy link

in PPC64LE system leapp upgrade took around 2 hrs 30 mins



As per below logs the udevadm_info start time 14:11:21 and end time 16:07:09 time taken approximately 2 Hours which is too long.

$ cat var/log/leapp/leapp-upgrade.log | grep udevadm_info
2022-03-19 14:11:21.148 INFO PID: 162587 leapp.workflow.FactsCollection: Executing actor udevadm_info <---- Started
...
...
...
2022-03-19 16:07:09.198 DEBUG PID: 42064 leapp.workflow.FactsCollection.udevadm_info: External command has finished: ['udevadm', 'info', '-e'] <---Ended

$ cat var/log/leapp/leapp-upgrade.log | grep udevadm_info | wc -l
55717 (approx 56K lines)

@pirat89
Copy link
Member

pirat89 commented Mar 28, 2022

@anandanramadoss Hi, in case you still see the problem, report please new issue with all required information inside the template. But first, please ensure you have installed up-to-date versions of leapp & leapp-repository (leapp-upgrade-*) packages and that you have set the expected devel envars. As well, if you test with the current upstream builds (leapp 0.14.0; leapp-repository-0.16.0) the behaviour should be default for the phases until the first reboot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants