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

Lock Annotation #135

Open
smillidge opened this issue Mar 17, 2021 · 21 comments
Open

Lock Annotation #135

smillidge opened this issue Mar 17, 2021 · 21 comments
Labels
EE12 enhancement New feature or request
Milestone

Comments

@smillidge
Copy link
Contributor

Create a Lock annotation that is functionally equivalent to the EJB Lock annotation but is more general in scope and therefore can be used more generally across the platform.

see https://docs.google.com/document/d/1uZFBoIujXCc-gQhCzh_ZdlKEsrsV0yVVIHzBTI3usF8/edit#heading=h.90fmh0qb8iq4 and https://jakarta.ee/specifications/platform/9/apidocs/jakarta/ejb/lock

@smillidge smillidge added this to the 3.0 milestone Mar 17, 2021
@tkburroughs
Copy link
Member

tkburroughs commented May 6, 2021

Should an XML equivalent also be defined? Or would that be left to the discretion of the specifications that choose to support it?

For example, the EJB specification supports the following XML equivalent of the annotation in ejb-jar.xml:

    <concurrent-method>
        <method>
            <method-name>beanMethod1</method-name>
        </method>
        <lock>Write</lock>
    </concurrent-method>
    <concurrent-method>
        <method>
            <method-name>beanMethod2</method-name>
        </method>
        <lock>Read</lock>
    </concurrent-method>

Other specification that might want to use this annotation, and also have XML (like web.xml) may also desire to have an XML equivalent.

If the annotation were the same as EJB, except with a different package... i.e. so only supporting 2 options:

@Lock(LockType.READ)
@Lock(LockType.WRITE)

With a default of WRITE, then perhaps the EJB spec could just allow applications to use the EJB version and new concurrency version interchangeably, then the above specific EJB XML could be left alone; no need to support two different ways in XML.

I assume the EJB specification would be allowed to consume the new annotation and :

  • limit use to only specific bean types (i.e only singleton)
  • ignore the annotation if present on an unsupported bean type, or interface
  • ignore the annotation if the module XML file contains metadata-complete="true"
  • use the same inheritance and override rules currently in the EJB specification

@smillidge
Copy link
Contributor Author

I haven't thought too much on how other specs consume the api. As a first instance I think it would be good for it to behave similar to the EJB annotations so that they can be used interchangeably with the advantage that it brings the equivalent EJB functionality to CDI beans.

@njr-11 njr-11 modified the milestones: 3.0, 3.1 Jan 13, 2022
@smillidge smillidge added the enhancement New feature or request label Feb 8, 2023
@hantsy
Copy link

hantsy commented May 25, 2023

  • The lock is available in a distributed env? When the application is scaled horizontally, all instances can share a central storage.

  • If possible to allow developers to expose a custom LockRegistry and the @Lock annotation can specify a lock name and backend lock registry?

  • It provides a programmatic API instead of the @Lock annotation.

    // perform operation in a closure, lock and unlock automatically.
    locker.withLock{}
    
    // or 
    locker.arquireLock();
    try{}
    finally{locker.releaseLock()}
  • It provides a Pool concept, like Kotlin Semaphore,https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines.sync/-semaphore/

@arjantijms
Copy link
Contributor

Should an XML equivalent also be defined?

Let's not do that.

I assume the EJB specification would be allowed to consume the new annotation

I don't think we should update EJB anymore, and just leave it be.

At most we can define a kind of subset of EJB lite that consists of a set of annotations and extensions that lets EJB be implemented via CDI. This new annotation would then automatically apply to these 'EJB' beans (which are actually CDI beans then).

See https://github.com/OmniFish-EE/omni-beans for a prototype of that idea.

@tkburroughs
Copy link
Member

While omni-beans is an interesting exercise to demonstrate how closely CDI has come to replicating EJB functionality, any product that ships the EJB API classes would be required to pass the EJB TCK, and would therefore be an EJB implementation (even if that is backed by a CDI implementation under the covers).

While we could update the EJB specification to define a new EJB Extra Lite, I think that would generally result in confusion with people wondering why they would want to use the new EJB Extra Lite vs just CDI. Or, are there still some remaining EJB exclusive capabilities that CDI really doesn't want?

The purpose of my prior comment was to:

  1. Identify some of the things EJB did with the @lock annotation for consideration. I agree that providing an XML equivalent should not be done.... just some of the baggage that comes along with EJB, which makes EJB undesirable.

  2. Note that some users might assume they could just use (or may accidentally use) the new @lock annotation with EJB. I think the original intent for Java EE was to have the ManagedBeans spec at the core, then CDI as an extension to that, and finally EJB as an extension to CDI. I'd prefer we strive to have only one bean model in Jakarta EE, which is CDI, and work toward completely eliminating ManagedBeans and EJB. So, while we might not update the EJB specification for this, it may be worth updating the Platform spec (or whatever spec contains the new @lock annotation) to specifically indicate it does not apply to either the Managed Beans or EJB specifications. Otherwise we could potentially end up with some EJB implementation providers supporting in and some not.

@m-reza-rahman
Copy link

XML is really not needed. We should try not to bother with EJB/managed beans any more and just stick to CDI as much as possible.

@arjantijms
Copy link
Contributor

Managed beans has been removed from the platform, and CDI no longer depends on it.

Still, there's a slew of remaining usage or references to this managed beans spec. For instance, Servlets are still managed beans, and REST still mentions managed beans in its spec document as well.

@arjantijms
Copy link
Contributor

I would like to propose still adding the Lock annotation for the 3.1 release. Don't go overboard yet with new features, but standardise what we done and understood through the years:

@arjantijms
Copy link
Contributor

I think that would generally result in confusion with people wondering why they would want to use the new EJB Extra Lite vs just CDI.

Compatibility with existing code mostly. We're (seemingly?) not yet ready to drop EJB completely from the profiles, probably because of the large body of existing code.

are there still some remaining EJB exclusive capabilities that CDI really doesn't want?

Most things in EJB Lite could be done by features in other specs, though most would land in Concurrency I think, and a few in Security. Transactions has already absorbed the EJB features, and Interceptors have already been split out. I don't think CDI itself (CDI core) wants any of those higher level features.

EJB Ultra / Extra Lite being based on CDI by spec definition, and not just as an implementation detail, might make a lot of things a lot easier and consistent in the platform, and make a level of legacy EJB support trivial to add to other platform such as Tomcat and Quarkus.

@njr-11
Copy link
Contributor

njr-11 commented Mar 11, 2024

I would like to propose still adding the Lock annotation for the 3.1 release. Don't go overboard yet with new features, but standardise what we done and understood through the years:

My vote is that we are too late in the Jakarta EE 11 release cycle for it to be a good idea to add more features. I would recommend that we look into adding it early on in 3.2. The focus for 3.1 should be on getting an RC1 built and ensure we have at least one compatible implementation passing the TCK by the approaching deadline for component specification release review. It should be noted that if Open Liberty is needed to be the compatible implementation, our schedules are such that the RC1 will be needed by March 20 (around 1 week from now), in order to have a publicly available downloadable beta that can be used to pass the TCK by the component specification deadline. If Glassfish can be used instead, maybe their schedules will allow some additional weeks of time, but it seems to me like we are inviting unnecessary risk if we choose to add anything new at this point in the schedule.

@arjantijms
Copy link
Contributor

My vote is that we are too late in the Jakarta EE 11 release cycle for it to be a good idea to add more features

I was indeed thinking about whether it would be too late or not. Concurrency is a bit later in the release waves, so theoretically there is some time, but let's look at it for 3.2 then.

@arjantijms arjantijms modified the milestones: 3.1, 3.2 Mar 14, 2024
@arjantijms arjantijms added the EE12 label Jan 7, 2025
@njr-11
Copy link
Contributor

njr-11 commented Jan 15, 2025

Some additional background:
EJB also has AccessTimeout which goes along with @Lock to control how long to block. It looks like Quarkus added its own Lock annotation that combines the two.

@m-reza-rahman
Copy link

The Quarkus design looks just about right, including basing it on ReadWriteLock. Having a separate AccessTimeout was always a little weird.

@hantsy
Copy link

hantsy commented Jan 21, 2025

@m-reza-rahman Instead of the simple timeout and throwing exceptions, I would like a configurable waiting mechanism and put the waiting instances into a queue or something.

@njr-11
Copy link
Contributor

njr-11 commented Jan 21, 2025

@m-reza-rahman Instead of the simple timeout and throwing exceptions, I would like a configurable waiting mechanism and put the waiting instances into a queue or something.

If we allow configuring a timeout, it would be optional to configure and would otherwise not time out. If we are basing this on ReadWriteLock and its provided implementation, ReentrantReadWriteLock, then the options for waiting mechanism are the Fair mode or Non-fair mode described in https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/concurrent/locks/ReentrantReadWriteLock.html . Is that what you had in mind when you say waiting mechanism? Or something different?

@m-reza-rahman
Copy link

If we allow configuring a timeout, it would be optional to configure and would otherwise not time out. If we are basing this on ReadWriteLock and its provided implementation, ReentrantReadWriteLock, then the options for waiting mechanism are the Fair mode or Non-fair mode.

I think this is more than fine. After all, the EJB locking functionality is pretty minimal. The Quarkus design also appears pretty simple.

@hantsy
Copy link

hantsy commented Jan 22, 2025

@njr-11 It looks good. I also hope there is a configurable provider to allow developers to use custom storage, such as Redis. When the application is scaled horizontally, all instances can share a central storage.

@njr-11
Copy link
Contributor

njr-11 commented Jan 22, 2025

@njr-11 It looks good. I also hope there is a configurable provider to allow developers to use custom storage, such as Redis. When the application is scaled horizontally, all instances can share a central storage.

Before we can even get to the topic of the possibility of applying a Lock across multiple servers like that, I would like to first figure out what the scope of a Lock is within a single server.

The CDI beans with methods to which Lock is applied will themselves have a scope, such as ApplicationScoped, RequestScoped, and so forth. In a single server, is the intention to have the locking happen at the bean instance level? Or at the annotated class/method level? If the former, are we planning to allow Lock to be used on CDI beans at all scopes, or limiting to a subset such as only ApplicationScoped and Singleton? Whichever way we end up choosing, I worry that if users make opposite assumptions about the scope of Lock, they will end up coding their applications incorrectly without realizing it.

@m-reza-rahman
Copy link

m-reza-rahman commented Jan 22, 2025

The primary intended use was indeed just application scoped beans and singletons, essentially at the instance level. However, it would be good to keep it as broadly applicable as possible.

I would suggest looking at what OmniServices and Quarkus is doing already (I think both just leave it open to users where they can apply @lock but provide some sort of guidance).

Java/Jakarta EE typically has left cluster-level concerns to vendors. I would just stick to standardizing single server behavior.

@njr-11
Copy link
Contributor

njr-11 commented Jan 22, 2025

The primary intended use was indeed just application scoped beans and singletons, essentially at the instance level. However, it would be good to keep it as broadly applicable as possible.

Thanks - that's good. It eliminates the ambiguity which would be the source of user errors related to scope if we limit to ApplicationScoped and Singleton, so I would definitely favor that. I would say wait until someone presents a good use case why they need Lock at another scope before opening it up further.

@m-reza-rahman
Copy link

m-reza-rahman commented Jan 22, 2025

That's fine. I would leave the possibility open for the future. There may be use cases I have not yet encountered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EE12 enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants