#1274 Don't make unnecessary rebuilds
Closed: Fixed 4 years ago by mprahl. Opened 4 years ago by vondruch.

Recently, there were done two small changes in Ruby modulemd file:

https://src.fedoraproject.org/modules/ruby/c/1e6f5f80fce62b5f7c2b533a8085f2a16973acbb?branch=master
https://src.fedoraproject.org/modules/ruby/c/09fa9ca804db58d0fdc457ac00a18b2f76d28ef1?branch=master

These are essentially just metadata changes. I don't understand, why every component was rebuilt after that. I don't think that was necessary, when rebuild_strategy "only-changed" was used:

https://mbs.fedoraproject.org/module-build-service/2/module-builds/4416


What does rebuild_strategy "only-changed" even affect?

When you submitted 4416, was the "master" of each component in your module pointing to the very same commit hash as in the pre-4416 build of your module?

When you submitted 4416, was the "master" of each component in your module pointing to the very same commit hash as in the pre-4416 build of your module?

I thought that too. As a result, the module-build-service skips the build.

I would prefer rebuild_strategy "skip-build" as the name.
That is inspired from Travis CI's skipping build feature.
https://docs.travis-ci.com/user/customizing-the-build/#skipping-a-build

In case of Travis CI, when people set below kind of commit message, they can skip the build.

[skip travis] Update README

I will have to think about it. The skip-build could break things really. You could put different hash in a ref, set skip-build and it would reuse component from previous module build no matter if it changed or not. The resulting module-build metadata would then not describe what is actually in the module.

If the module build should reuse previous component builds, the component builds must be the same. Some tool to set the "ref" of your current module to match the real commit hashes used to build the previous module could fix this.

If the module build should reuse previous component builds, the component builds must be the same. Some tool to set the "ref" of your current module to match the real commit hashes used to build the previous module could fix this.

@jkalina yeah, that looks a good logic.

Okay to clarify (I am a simpleton) in order for MBS to re-use builds in the "only-changed" strategy, I have to set an explicit hash as the git ref in the yaml file?

Right now I use "master" or the name of the branch, and it always rebuilds every component every time I change the module yaml file.

Okay to clarify (I am a simpleton) in order for MBS to re-use builds in the "only-changed" strategy, I have to set an explicit hash as the git ref in the yaml file?
Right now I use "master" or the name of the branch, and it always rebuilds every component every time I change the module yaml file.

That shouldn't be the case. MBS resolves the git ref to a commit hash and stores it in the modulemd's xmd field. This is what it uses to make the comparison.

Okay to clarify (I am a simpleton) in order for MBS to re-use builds in the "only-changed" strategy, I have to set an explicit hash as the git ref in the yaml file?
Right now I use "master" or the name of the branch, and it always rebuilds every component every time I change the module yaml file.

That shouldn't be the case. MBS resolves the git ref to a commit hash and stores it in the modulemd's xmd field. This is what it uses to make the comparison.

But that is what is happening... Consider my builds of the "eclipse" module.

From module build 4471:
https://koji.fedoraproject.org/koji/taskinfo?taskID=35234104

From module build 4551:
https://koji.fedoraproject.org/koji/taskinfo?taskID=35317424

Note how the git commit hash is identical for both builds. Why did it rebuild this component?

I assume I must be doing something stupid :-) Any hints?

4471 is failed. We currenly have RFE for reusing components also from failed builds. Currently, we can reuse only from "ready" module builds.

4439 does not buildrequire "tycho" module while 4457 buildrequires tycho module, 4457 therefore cannot reuse components from 4439, because it has different buildroot.

@jkaluza Any documentation of all these exceptions where the rebuild strategy does not apply?

4439 does not buildrequire "tycho" module while 4457 buildrequires tycho module, 4457 therefore cannot reuse components from 4439, because it has different buildroot.

Another reason to allow bootstrapping in a single pass (https://pagure.io/fm-orchestrator/issue/1267)

@mbooth, would you be okay if we opened a ticket requesting documentation on the rebuild strategies and then closed this ticket?

@mprahl huh, closing the ticket? I don't think my initial comment was resolved.

@mprahl huh, closing the ticket? I don't think my initial comment was resolved.

I'm sorry. I lost track of the original discussion in the ticket.

@vondruch the logs say the commit hash on the head of the master branch for those components changed since the last build of that component, so that's why a subset of your components couldn't be reused.

I looked into rubygem-mysql2 in particular, and saw that there was a commit on May 23rd:
https://src.fedoraproject.org/rpms/rubygem-mysql2/c/cc15309b2ad9c2b725b0df66e3e9b276f0653b5a?branch=master

The last build of that component for a module that buildrequired platform:f31, as did your module, was on April 30th:
https://koji.fedoraproject.org/koji/taskinfo?taskID=34552650

That component build was based on this commit:
https://src.fedoraproject.org/rpms/rubygem-mysql2/c/e274528d03b8788d8e84bc4de8fbc1e0db3d0327?branch=master

I understand component reuse can be confusing, and perhaps we can turn this ticket into an RFE to add a JSON field that lists the reason why a component wasn't reused. Then the next time your component isn't reused, you can verify the reason why. If the reason is wrong, we can then fix the underlying issue.

Looking at the matters once again, I was probably just confused.

and perhaps we can turn this ticket into an RFE to add a JSON field that lists the reason why a component wasn't reused.

Yes. There is already:

state_reason "Reused component from previous module build"

So if this could be extended to cover why it was not reused, that would be helpful. If there was even more information, such as hash of the current/reused build commit, that would help even more.

Also if the logs you mention:

the logs say the commit hash on the head of the master branch for those components changed since the last build of that component,

were accessible, that would help as well. I wonder, why we don't have MBS Web enabled?

@jkaluza, @mprahl

Hi, I have one more situation that I'd like to see explained before this becomes a documentation ticket:

Let's go back to my Eclipse module....

Successfully completed build 4610:
https://koji.fedoraproject.org/koji/taskinfo?taskID=35429116

Next build was build 4661:
https://koji.fedoraproject.org/koji/taskinfo?taskID=35467245

Once again, the identical git hash was built.

And this time there was no changes to the buildroot -- nothing changed in the module Rs/BRs between these two builds.

Edit: Changes made to the module yaml file between these two builds were two-fold: Add new RPMs to the "components" section and add a new RPM macro to the "buildopts" section. Should I expect to build everything from scratch once again in this case?

@mbooth I took a look at the logs and the reason that the components weren't reused is that the components are different in the old module build and the new module build [1]. I don't actually know why that makes it so that a component can't be reused. A lot of these decisions around component reuse were made 2+ years ago. @psabata, do you have any opinions on this? Is this something we can change?

1 - https://pagure.io/fm-orchestrator/blob/master/f/module_build_service/utils/reuse.py#_363

Looking at the matters once again, I was probably just confused.

and perhaps we can turn this ticket into an RFE to add a JSON field that lists the reason why a component wasn't reused.

Yes. There is already:
state_reason "Reused component from previous module build"
So if this could be extended to cover why it was not reused, that would be helpful. If there was even more information, such as hash of the current/reused build commit, that would help even more.
I filed #1284

Also if the logs you mention:

the logs say the commit hash on the head of the master branch for those components changed since the last build of that component,

were accessible, that would help as well. I wonder, why we don't have MBS Web enabled?
Could you please clarify what you mean by "why we don't have MBS Web enabled"?

The logs are public when they are uploaded to the Koji content generator build representing the module, but I realize this isn't very user-friendly and this wouldn't cover failed builds.

@mbooth I took a look at the logs and the reason that the components weren't reused is that the components are different in the old module build and the new module build [1]. I don't actually know why that makes it so that a component can't be reused. A lot of these decisions around component reuse were made 2+ years ago. @psabata, do you have any opinions on this? Is this something we can change?
1 - https://pagure.io/fm-orchestrator/blob/master/f/module_build_service/utils/reuse.py#_363

The idea behind this code was that if you add component to module build, it can influence other components (for example adding "openssl" to "httpd" module can make "httpd" to find out "openssl" and build "mod_ssl") and therefore we cannot reuse such components from older module build.

I think with "only-changed" rebuild strategy, this check could be removed maybe.

@mbooth I took a look at the logs and the reason that the components weren't reused is that the components are different in the old module build and the new module build [1]. I don't actually know why that makes it so that a component can't be reused. A lot of these decisions around component reuse were made 2+ years ago. @psabata, do you have any opinions on this? Is this something we can change?
1 - https://pagure.io/fm-orchestrator/blob/master/f/module_build_service/utils/reuse.py#_363

The idea behind this code was that if you add component to module build, it can influence other components (for example adding "openssl" to "httpd" module can make "httpd" to find out "openssl" and build "mod_ssl") and therefore we cannot reuse such components from older module build.
I think with "only-changed" rebuild strategy, this check could be removed maybe.

Wait -- Now I am even more confused..... Because that rationale doesn't make any sense given the current behaviour.....

E.g.: I just rebuilt my tycho module: https://release-engineering.github.io/mbs-ui/module/4718

It only rebuilt "jetty", "eclipse-emf", "eclipse-ecf" and "eclipse" components (as I expected because only these components changed). But the components that come later in the build order were not rebuilt, but instead reused from the previous build, even though any of the rebuilt components could influence the builds of the components that come later in the build order.

What's going on here? If you are concerned that adding a component could affect the build of another component, then surely if a component with buildorder = 40 changes and is rebuilt then all components with a buildorder > 40 should also be rebuilt?

IMO it only makes sense to re-use components that come before changed/added components in the buildorder.

@mbooth I took a look at the logs and the reason that the components weren't reused is that the components are different in the old module build and the new module build [1]. I don't actually know why that makes it so that a component can't be reused. A lot of these decisions around component reuse were made 2+ years ago. @psabata, do you have any opinions on this? Is this something we can change?
1 - https://pagure.io/fm-orchestrator/blob/master/f/module_build_service/utils/reuse.py#_363
The idea behind this code was that if you add component to module build, it can influence other components (for example adding "openssl" to "httpd" module can make "httpd" to find out "openssl" and build "mod_ssl") and therefore we cannot reuse such components from older module build.
I think with "only-changed" rebuild strategy, this check could be removed maybe.

Wait -- Now I am even more confused..... Because that rationale doesn't make any sense given the current behaviour.....
E.g.: I just rebuilt my tycho module: https://release-engineering.github.io/mbs-ui/module/4718
It only rebuilt "jetty", "eclipse-emf", "eclipse-ecf" and "eclipse" components (as I expected because only these components changed). But the components that come later in the build order were not rebuilt, but instead reused from the previous build, even though any of the rebuilt components could influence the builds of the components that come later in the build order.
What's going on here? If you are concerned that adding a component could affect the build of another component, then surely if a component with buildorder = 40 changes and is rebuilt then all components with a buildorder > 40 should also be rebuilt?

The behavior you described would be for the changed-and-after rebuild strategy which is more conservative. It's documented in the read me in the "Rebuild Strategies" section, but we need to add more details to it.

@mbooth I took a look at the logs and the reason that the components weren't reused is that the components are different in the old module build and the new module build [1]. I don't actually know why that makes it so that a component can't be reused. A lot of these decisions around component reuse were made 2+ years ago. @psabata, do you have any opinions on this? Is this something we can change?
1 - https://pagure.io/fm-orchestrator/blob/master/f/module_build_service/utils/reuse.py#_363

The idea behind this code was that if you add component to module build, it can influence other components (for example adding "openssl" to "httpd" module can make "httpd" to find out "openssl" and build "mod_ssl") and therefore we cannot reuse such components from older module build.
I think with "only-changed" rebuild strategy, this check could be removed maybe.

After discussing with @psabata today, I filed the following ticket to address this:
https://pagure.io/fm-orchestrator/issue/1298

FYI, I filed this PR to add additional documentation about rebuild strategies:
https://pagure.io/fm-orchestrator/pull-request/1304

@vondruch is it okay if we close this ticket in favor of the tickets/PRs listed above?

@mprahl yes please. I'll open new one if I suspect that something is built when it should not be ;) Thx

Metadata Update from @mprahl:
- Issue close_status updated to: Fixed
- Issue status updated to: Closed (was: Open)

4 years ago

Login to comment on this ticket.

Metadata