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

isAsync takes precedence over OperationHostileThread #26227

Closed
wants to merge 1 commit into from

Conversation

lukasherman
Copy link
Contributor

Fixes #24712
OperationExecutor allows async invocation of OperationHostileThread.

@hz-devops-test hz-devops-test added the Source: Community PR or issue was opened by a community user label Jan 23, 2024
@devOpsHazelcast
Copy link
Collaborator

Can one of the admins verify this patch?

@devOpsHazelcast
Copy link
Collaborator

devOpsHazelcast commented Jan 23, 2024

CLA assistant check
All committers have signed the CLA.

@devOpsHazelcast
Copy link
Collaborator

Can one of the admins verify this patch?

1 similar comment
@devOpsHazelcast
Copy link
Collaborator

Can one of the admins verify this patch?

@devOpsHazelcast
Copy link
Collaborator

Internal PR hazelcast/hazelcast-mono#484
Internal message only. Nothing to see here, move along

@TomaszGaweda
Copy link
Contributor

@vbekiaris can you please take a look?

@JamesHazelcast
Copy link
Contributor

Thanks for submitting this contribution. I don't see any obvious downsides to this change, but the interface OperationHostileThread suggests that these threads should never be used to run Operations, in which case it makes sense to disallow running Operations on these threads, even if called async.

As I'm not familiar with OperationHostileThread and its exact usage, I'll call in the expert @pveentjer for his review of this change.

@pveentjer
Copy link
Contributor

pveentjer commented Apr 9, 2024

The purpose of OperationHostileThread is to prevent any operation running on that thread. For partition-specific operations, it is obvious that you need to run on a partition-thread, but there are also operations that do not require a partition thread to run on. We need to make sure that such operations don't get executed on such an OperationHostileThread (e.g. an IO-thread).

@pveentjer
Copy link
Contributor

The change is fine since with 'async' the actual operation execution will always be offloaded to the right thread and will never be executed on an OperationHostileThread.

Copy link
Contributor

@JamesHazelcast JamesHazelcast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution @lukasherman, we appreciate it!

@devOpsHazelcast devOpsHazelcast added this to the 5.5.0 milestone Apr 9, 2024
@zjedi
Copy link

zjedi commented Apr 9, 2024

any chance for backporting the fix to 5.3 branch?

@JamesHazelcast
Copy link
Contributor

CLA assistant check Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

Lukáš Herman seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Please sign the CLA so we can proceed @lukasherman.

@lukasherman
Copy link
Contributor Author

Please sign the CLA so we can proceed @lukasherman.

I have signed it several times already, please check.

@devOpsHazelcast
Copy link
Collaborator

This pull request has been closed because it was already merged as https://github.com/hazelcast/hazelcast-mono/pull/484

@devOpsHazelcast devOpsHazelcast modified the milestones: 5.5.0, 5.3.z Apr 10, 2024
@JamesHazelcast
Copy link
Contributor

any chance for backporting the fix to 5.3 branch?

This will be backported to our 5.3.z branch, yes 👍

devOpsHazelcast pushed a commit that referenced this pull request Apr 10, 2024
Import of #26227

**Original PR description:**

Fixes #24712
OperationExecutor allows async invocation of OperationHostileThread.

Imported changes:

- ad43e41 isAsync takes precedence over
OperationHostileThread

Closes #26227

Co-authored-by: Lukáš Herman <lukas.herman@cgi.com>
GitOrigin-RevId: ed32fc9cca084cfc14ad6b846931fbd3798b6820
@devOpsHazelcast devOpsHazelcast modified the milestones: 5.3.z, 5.4.z Apr 10, 2024
devOpsHazelcast pushed a commit that referenced this pull request Apr 10, 2024
Forwardport of https://github.com/hazelcast/hazelcast-mono/pull/484

Import of #26227

**Original PR description:**

Fixes #24712
OperationExecutor allows async invocation of OperationHostileThread.

Imported changes:

- ad43e41 isAsync takes precedence over
OperationHostileThread

Co-authored-by: Lukáš Herman <lukas.herman@cgi.com>
GitOrigin-RevId: c80a5d09f3dd556d1fa24dead95d17f5322eb36f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants