Skip to content
This repository has been archived by the owner on Apr 21, 2023. It is now read-only.

Building twice because of new "touch on recovery" #1710

Closed
Bananeweizen opened this issue Aug 3, 2021 · 9 comments · Fixed by #1727
Closed

Building twice because of new "touch on recovery" #1710

Bananeweizen opened this issue Aug 3, 2021 · 9 comments · Fixed by #1727
Assignees
Milestone

Comments

@Bananeweizen
Copy link
Contributor

#1671 has led to the situation that projects are built twice in certain situations:

  • run eclipse with an empty workspace
  • import an Xtext project
  • watch the Xtext console, it is reported as being built twice
    (This is on Xtext 2.25 with the BuildStateDiscarder changes being ported back. But I'm quite sure the behaviour is identical on current Xtext).

The technical background is as follows:

  • Multiple operations (create project, open project, refresh workspace) of the SmartImportWizard trigger the needsBuild flag of the AutoBuild job, therefore a (full) build of the newly imported project is started.
  • The opening of the project schedules the Xtext recovery build, since this is the first Xtext project in the workspace.
  • The BuildStateDiscarder decides to discard the build state (because of the recovery build flag) and to schedule a job for touching the project.
  • The full build from the AutoBuild job finishes. Only now the scheduled job for touching the project is really started, because it uses the project as lock, and therefore cannot run concurrently to the AutoBuild triggered build. At this point of time the touch is superfluous, but it has been scheduled long ago.

I believe the exceptions and deadlocks reported in the early versions of #1671 (when the touch was executed immediately instead of via job) were also caused by the touch happening when the auto build already built the same project.

The most simple fix would be to check again inside the touch job run() method whether touching is actually still needed. However, I did not find any good method to check whether a project has been built (via Xtext BuildManagerAccess). Any advice how that could be checked, or do you imagine another fix eventually?

@cdietrich
Copy link
Member

cc @szarnekow @tivervac

@cdietrich
Copy link
Member

maybe also andreys changes mentioned in https://bugs.eclipse.org/bugs/show_bug.cgi?id=549704 are related

@cdietrich
Copy link
Member

cdietrich commented Aug 6, 2021

with Xtext 2.26 nightly and eclipse 2021-09 nightly i see

Import

[Worker-6: Building] Building demo
[Worker-6: Building] Built demo in 2 ms
[Worker-2: Building] Building demo
[Worker-2: Building] indexing platform:/resource/demo/src/mydsl/MrsGrantsSecretCompartments.statemachine
[Worker-2: Building] Built demo in 152 ms
[Worker-2: Building] Building demo
[Worker-2: Building] Built demo in 0 ms

open

[Worker-10: Building] Building demo
[Worker-10: Building] indexing platform:/resource/demo/src/mydsl/MrsGrantsSecretCompartments.statemachine
[Worker-10: Building] Built demo in 100 ms
[Worker-10: Building] Building demo
[Worker-10: Building] Built demo in 0 ms

so there is one real full build only
@Bananeweizen @rytina
i am not sure if the reproduction steps are sufficient or if andreys fix actually fixed the problem

@rytina
Copy link

rytina commented Aug 9, 2021

i am not sure if the reproduction steps are sufficient or if andreys fix actually fixed the problem

@cdietrich Please make sure to import the Xtext project from an archive file into a clean workspace how it is demonstrated with https://drive.google.com/file/d/1N6ax6sTQ7EuvSA5rL537lPddasYXIgUf/view?usp=drivesdk
I can reproduce this bug with standard Eclipse 2021-06 where I installed org.eclipse.xtext.sdk.p2-repository-2.26.0-SNAPSHOT.zip using the default domain model example.

@cdietrich
Copy link
Member

interestingly i can see it with the domain model example but i could not see it with a statemachine example

[Worker-1: Building] 6
[Worker-1: Building] Building dully
[Worker-1: Building] indexing platform:/resource/dully/src/dully.dmodel
[Worker-1: Building] Built dully in 3828 ms
[Worker-7: Building] 6
[Worker-7: Building] Building dully
[Worker-7: Building] indexing platform:/resource/dully/src/dully.dmodel
[Worker-7: Building] Built dully in 570 ms
[Worker-3: Building] 9
[Worker-3: Building] Building dully
[Worker-3: Building] Built dully in 3 ms

will check next if the problem is zip import

@cdietrich
Copy link
Member

zip or not does not play a role
dm builds twice, statemachine not.

@cdietrich
Copy link
Member

with yet another mydsl i get

[Worker-1: Building] 6
[Worker-1: Building] Building myp
[Worker-1: Building] Built myp in 13 ms
[Worker-3: Building] 6
[Worker-3: Building] Building myp
[Worker-3: Building] indexing platform:/resource/myp/demo.mydsl1
[Worker-3: Building] Built myp in 835 ms

=> is build cancelled?!?

@Bananeweizen
Copy link
Contributor Author

@cdietrich I'm not sure but I suspect that very small (and fast built) models might not trigger the bug simply because it requires some "timely overlap" from the auto build job and the xtext refresh job. As far as I remember, the auto build job itself uses a randomized schedule delay of around 1 second, therefore you probably need models of at least 1 or 2 seconds build time to trigger it.
I haven't investigated when the "needsBuild" flag of the auto build is cleared, but maybe in your case the build from the refresh trigger cleared that flag before the auto build job was even scheduled.

@szarnekow szarnekow self-assigned this Aug 10, 2021
@szarnekow szarnekow added this to the Release_2.26 milestone Aug 10, 2021
szarnekow added a commit that referenced this issue Aug 21, 2021
The window of opportunity for the autobuild is the time between
forgetting the last built state and the touch of the project in its own
job. Check if there is a known built state before touching the project
again.

Signed-off-by: Sebastian Zarnekow <[email protected]>
szarnekow added a commit that referenced this issue Aug 21, 2021
The window of opportunity for the autobuild is the time between
forgetting the last built state and the touch of the project in its own
job. Check if there is a known built state before touching the project
again.

Signed-off-by: Sebastian Zarnekow <[email protected]>
szarnekow added a commit that referenced this issue Aug 22, 2021
@szarnekow
Copy link
Contributor

@cdietrich Can you by chance take another look?

szarnekow added a commit that referenced this issue Aug 23, 2021
The strategy for the recovery build was changed such that we keep
an additional flag on the builder to indicate that the next regular
build is supposed to be a full build because the resource deltas cannot
be trusted.

Signed-off-by: Sebastian Zarnekow <[email protected]>
szarnekow added a commit that referenced this issue Aug 23, 2021
The strategy for the recovery build was changed such that we keep
an additional flag on the builder to indicate that the next regular
build is supposed to be a full build because the resource deltas cannot
be trusted.

Signed-off-by: Sebastian Zarnekow <[email protected]>
szarnekow added a commit that referenced this issue Aug 23, 2021
The strategy for the recovery build was changed such that we keep
an additional flag on the builder to indicate that the next regular
build is supposed to be a full build because the resource deltas cannot
be trusted.

Signed-off-by: Sebastian Zarnekow <[email protected]>
szarnekow added a commit that referenced this issue Aug 23, 2021
The strategy for the recovery build was changed such that we keep
an additional flag on the builder to indicate that the next regular
build is supposed to be a full build because the resource deltas cannot
be trusted.

Signed-off-by: Sebastian Zarnekow <[email protected]>
szarnekow added a commit that referenced this issue Sep 22, 2021
The strategy for the recovery build was changed such that we keep
an additional flag on the builder to indicate that the next regular
build is supposed to be a full build because the resource deltas cannot
be trusted.

Signed-off-by: Sebastian Zarnekow <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants