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

Optimised integration between Context Propagation and Mutiny #20743

Merged
merged 1 commit into from
Oct 13, 2021

Conversation

Sanne
Copy link
Member

@Sanne Sanne commented Oct 13, 2021

This unlocks significant performance improvements as it allows to skip CP processing in a Mutiny chain, following the existing documentation we have in the CP guide.

(Apparently it wasn't being applied to Mutiny - not sure if to classify it as a bugfix, let's say it's an efficiency improvement)

@Sanne Sanne requested review from jponge and FroMage October 13, 2021 14:53
@quarkus-bot quarkus-bot bot added the area/dependencies Pull requests that update a dependency file label Oct 13, 2021
@Sanne Sanne added priority/urgent area/dependencies Pull requests that update a dependency file and removed area/dependencies Pull requests that update a dependency file labels Oct 13, 2021
@gastaldi gastaldi added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 13, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Oct 13, 2021

Failing Jobs - Building ad2cd96

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
JVM Tests - JDK 11 Windows Upload gc.log ⚠️ Check → Logs Raw logs
✔️ JVM Tests - JDK 17

@Sanne
Copy link
Member Author

Sanne commented Oct 13, 2021

@geoand , @famod : there's an odd failure on a windows specific github action... but it might be related :/ Do you know about that task?

I see it has the context propagation 1.2 in the path, which my PR updated to 1.2.2

No idea how this triggered it, but I see the github action is:

     - name: Upload gc.log
        uses: actions/upload-artifact@v2
        with:
          name: "GC log - JDK ${{matrix.java.name}}"
          path: |
            **/windows-java-11.txt
            !**/build/tmp/**

While the errror message states:

Error: ENOENT: no such file or directory, realpath 'D:\a\quarkus\quarkus\devtools\gradle\gradle-extension-plugin\build\tmp\test\work\.gradle-test-kit\caches\modules-2\metadata-2.97\descriptors\org.eclipse.microprofile.context-propagation\microprofile-context-propagation-parent\1.2\a8be1fe3b3911d3d3425fe720cf42835'

Is it possible that last line in the path exclusions needs to be expressed in windows format? I'm just guessing, really not familiar with this.

@Sanne
Copy link
Member Author

Sanne commented Oct 13, 2021

Looks like on windows slashes can be expressed in both ways (according to docs at least..)

But also, a missing file should generate a warning by default:

So I'm not sure. I wonder if it's racy: the gradle cache daemon could have deleted the file after the expression had included it, but before it could have finished the transfer? Since it failed on Windows, I'm not surprised an opened file wasn't locked against deletion.

But then again the exlusion pattern should have prevented this.. I'm confused.

@Sanne
Copy link
Member Author

Sanne commented Oct 13, 2021

oh, just found this is an existing problem:

The relation with context-propagation was a coincidence apparently .. I guess I'll merge then!

@Sanne Sanne merged commit ee36cbd into quarkusio:main Oct 13, 2021
@quarkus-bot quarkus-bot bot added this to the 2.5 - main milestone Oct 13, 2021
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 13, 2021
@Sanne Sanne deleted the CPOptimisations branch October 13, 2021 21:09
@aloubyansky aloubyansky modified the milestones: 2.5 - main, 2.4.0.Final Oct 18, 2021
@famod
Copy link
Member

famod commented Oct 18, 2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependencies Pull requests that update a dependency file kind/enhancement New feature or request priority/urgent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants