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

Add missing Cache Flags for AsPool #382

Closed
wants to merge 1 commit into from
Closed

Add missing Cache Flags for AsPool #382

wants to merge 1 commit into from

Conversation

BernardoGomesNegri
Copy link

A simple patch that I believe will help fix KDE bug #443555 Even if this part of the library is deprecated, it is still useful to have for some projects.

Even if this part of the library is deprecated, it is still useful to have for some projects. I also believe this will help fix KDE bug #443555
@ximion
Copy link
Owner

ximion commented Feb 20, 2022

Can you explain how adding this would help with the linked bug? This has never been part of the ASQt API, and setting any of these flags does exactly nothing (those flags are completely ignored by AsPool), so as far as I can see, all this patch does is adding some deprecated API that we will remove at the next opportunity again anyway.

@BernardoGomesNegri
Copy link
Author

Upon digging through plasma-discover's source code, I am fairly certain that it is due to the cache being cleared and rewritten (when diagnosing the bug, I found very high disk write, about 2GB/s for 4 minutes when the cache file is about half a Megabyte, but the only open file was the Appstream cache file and it was not increasing in size and the metadata was very recent). If you would like, I could try to reproduce it with the patched library.
Plus, both versions of the API matching is always good, especially when there is no way to access the internal GObject from the QT library.

@ximion
Copy link
Owner

ximion commented Feb 20, 2022

Plus, both versions of the API matching is always good, especially when there is no way to access the internal GObject from the QT library.

Not if deprecated stuff is added to a library that didn't ever have it and that does absolutely nothing. Passing these flags to an AsPool does literally nothing except generating a deprecation warning. You never want to use this, ever.
Adding it will also not change how Discover operates, so I don't think I see the whole picture of what you are trying to achieve here yet.

What's your AppStream version that you use with Discover?

@BernardoGomesNegri
Copy link
Author

Right now I am using the version in the Fedora 35 repositories, 0.14.6, the CMake file of the repo asks for 0.14.4 but it is compiling fine on my machine.
If the flags do nothing, is there an alternative to not clear the cache?

@aleixpol
Copy link
Collaborator

I do not think this is related to the bug discussed, not that it's undesirable or unwanted. No developer has been able to reproduce that problem so I'd first like to see some investigation on its nature before we submit changes to see what sticks.

@BernardoGomesNegri
Copy link
Author

I do not think this is related to the bug discussed, not that it's undesirable or unwanted. No developer has been able to reproduce that problem so I'd first like to see some investigation on its nature before we submit changes to see what sticks.

I can consistently reproduce the bug on my machine by opening up Discover with the Flatpak backend enabled and I strongly believe the cache being is to blame. When I check for updates with the Flatpak backend, the size of the cache files immediately goes down to zero, then starts going up again. When the files stop going up in size, it shows what updates are avaliable.
If the flags really do absolutely nothing, then close this pull request as it has no point.

@ximion
Copy link
Owner

ximion commented Feb 21, 2022

@BernardoGomesNegri Which distribution are you using? Can you update AppStream to any version beyond 0.15?
(Also, I'd definitely not merge this PR as-is, but if there's a different bug we can probably work out what that is)

@ximion ximion closed this Feb 21, 2022
@BernardoGomesNegri
Copy link
Author

@BernardoGomesNegri Which distribution are you using? Can you update AppStream to any version beyond 0.15? (Also, I'd definitely not merge this PR as-is, but if there's a different bug we can probably work out what that is)

After updating to appstream 0.15.1 with the builds from https://koji.fedoraproject.org/koji/buildinfo?buildID=1880490 and changing the CMake file to ask for the newest version of appstream, the bug does not happen anymore. Thank you for the help. The bug should be fixed naturally as distros upgrade their repositories.

@ximion
Copy link
Owner

ximion commented Feb 21, 2022

What filesystem are you using, and what version of LMDB did you have installed?

@BernardoGomesNegri
Copy link
Author

What filesystem are you using, and what version of LMDB did you have installed?

ext4 on LVM and LMDB 0.9.29, the latest on the Fedora repos.
If you can, I'd recommend pushing the latest version of this library to the Fedora 35 repos to fix the Discover bug.

@ximion
Copy link
Owner

ximion commented Feb 21, 2022

Okay, so not a unusual configuration at all. This issue makes no sense.
I have no direct influence on Fedora, but they do keep AppStream very much up to date in their releases in general, so if it's not in the current release, it will certainly be updated in the next one.

@BernardoGomesNegri
Copy link
Author

It is scheduled that Fedora 36 will have appstream 0.15.1, so the bug should be fixed by then.

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.

None yet

3 participants