More UpdateReleasedBinariesJob Refactoring #16425
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hennevogel
force-pushed
the
refactoring/UpdateReleasedBinariesJob
branch
4 times, most recently
from
af577a3
to
3e8b25e
Compare
| # building a hash to avoid single SQL select calls slowing us down too much | ||
| # We record BinaryRelease attributes in this hash of hashes to be able to compare them later on. | ||
| # This is an optimization so we do not have to fetch BinaryRelease individually from the database. | ||
| # FIXME: This is exactly what ActiveRecord::Associations::CollectionProxy does no? |
There was a problem hiding this comment.
no, activerecord has always full objects in memory
There was a problem hiding this comment.
You talk about putting the full record into the hash, I talk about not using the hash at all...
| BinaryRelease.transaction do | ||
| # Populate the old_binary_releases hash | ||
| repository.binary_releases.current.unchanged.find_each do |binary| |
There was a problem hiding this comment.
the limit override is missing again, this will increase easily at least one hour of runtime again.
There was a problem hiding this comment.
What is a "limit override"?
| new_binary_release.binary_releasetime = time | ||
|
|
||
| # Set defaults for versions/release | ||
| # FIXME: This could also happen in the database instead... |
There was a problem hiding this comment.
we don't want to modify old database entries if possible.
There was a problem hiding this comment.
Not necessary as the default in the code is there since we create those entries...
adrianschroeter
requested changes
Jul 1, 2024
|
On Montag, 1. Juli 2024, 16:38:35 CEST Henne Vogelsang wrote:
@hennevogel commented on this pull request.
> BinaryRelease.transaction do
+ # Populate the old_binary_releases hash
repository.binary_releases.current.unchanged.find_each do |binary|
What is a "limit override"?
The .limit(0). from my speedup patch.
Otherwise the default 1000 limit leads to a really long time running loop.
…--
Adrian Schroeter ***@***.***>
Build Infrastructure Project Manager
SUSE Software Solutions Germany GmbH, Frankenstraße 146, 90461 Nürnberg, Germany
(HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev
|
Sorry, no idea what you are talking about. This is your original patch right? https://github.com/openSUSE/open-build-service/pull/16366/files |
hennevogel
force-pushed
the
refactoring/UpdateReleasedBinariesJob
branch
from
3e8b25e
to
849df05
Compare
With a little bit more realistic data.
Now that we have attribute aliases we don't need different methods to build the hash key anymore. BinaryRelease and the hash we receive from the backend have the same keys...
Instead of building a hash build a `BinaryRelease` directly using the backend hash.
This attribute is *always* assigned a value in the one and only place we ever create a BinaryRelease: UpdateReleasedBinariesJob set_release_time! was unsused throughout the code base...
It's only used once throughout the code base. Use a speaking variable instead.
Yes we process these "items" but the whole set of this will be the current BinaryRelease of Repository after we are done.
Clarify what is going on and why we do some things.
No need to store the whole object, we only need the ID.
Specs for flavor/binary_maintainer attribute assignment and specs for the release_package and on_medium associations that get built.
hennevogel
force-pushed
the
refactoring/UpdateReleasedBinariesJob
branch
from
849df05
to
8a3c10e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.