[DISCUSS] Deprecate query part of Sling Event API

classic Classic list List threaded Threaded
27 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[DISCUSS] Deprecate query part of Sling Event API

Stefan Egli-2
Hi,

I'd like to reactivate SLING-5884 and a related discussion [1] we had
about deprecating the query part of the Sling Event API.

TL;DR: Open question is exactly which of the JobManager methods do we
want to deprecate.


The idea is to deprecate (and sometime in the future - timing tbd -
remove) those methods of the sling event api which we either consider
expensive to support (eg maintaining large indexes for queries) or
making assumptions/limitations on the underlying implementations and
thus prohibit usable alternative implementations such as messaging-based
ones.


Now to the discussion as to what exactly we want to deprecate and how we
see alternatives (or not) for each (all methods of JobManager [2]):

(A) addJob, createJob: Those must stay

(B) getQueue(s), get(Topic)Statistics: Those can probably stay

(C) getJob, findJobs (and QueueType): those are the main query methods
that should be deprecated. We'd not provide an alternative, but could
suggest those who want to maintain job information could do that
themselves in a shared place.

(D) getJobById, retryJobById: both assume some kind of job storage that
can be accesses in an arbitrary way. Thus I'd argue both could be
deprecated. Alternative suggestion similar to (C). Edge-case.

(E) removeJobById: can imv stay. the api makes no hard requirement to
check if the job ever existed, however it requires that the job be
removed when returning true, so requires (expensive) synchronicity.
Doable. Edge-case.

(F) stopJobById: can imv stay.

(F) getScheduledJobs: while this requires some form of job storage it
only affects a small amount of data. But if we'd want to make the job
api messaging-ready it would also have to be deprecated. Edge-case.


Cheers,
Stefan
--
[0] - https://issues.apache.org/jira/browse/SLING-5884
[1] - http://sling.markmail.org/message/k3hjqcvnnabsb47j
[2] -
https://github.com/apache/sling-org-apache-sling-event-api/blob/master/src/main/java/org/apache/sling/event/jobs/JobManager.java
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Deprecate query part of Sling Event API

Robert Munteanu-2
Hi Stefan,

On Mon, 2018-10-29 at 14:47 +0100, Stefan Egli wrote:
> Hi,
>
> I'd like to reactivate SLING-5884 and a related discussion [1] we
> had
> about deprecating the query part of the Sling Event API.
>
> TL;DR: Open question is exactly which of the JobManager methods do
> we
> want to deprecate.


How about depreacting the JobManager completely and creating new API
which does exactly what we want?

We can switch the JobManagerImpl to being a component registered only
if a certain configuration property is set. We can set the default to
'false' and log a WARN when we register that component to make it clear
that it's not going to be supported. Eventually we can stop registering
the implementation outright and make the inconvenient methods throw an
UnsupportedOperationException. But I would prefer to create a new API
for new 'realities'.

I know the transition will be more inconvenient, but a new API will
also force clients to make conscious choices about what they want to
support and a clear transition path.

While not giving it a lot of thought to the names and the API methods
we could support, here's a strawman API we can bash:

----------------

// do we need both submit and create?
interface JobManager2 {
  SubmittedJob submitJob(...)
  JobBuilder createJob(...)
}

// do we need both stop and remove?
interface SubmittedJob {
  void retryJob()
  void removeJob()
  void stopJob()
}

interface JobStatisticsManager {
  getQueue(...);
  getQueues();
  getStatistics();
  // etc, etc
}

----------------

The separation of concerns makes it quite clear what is possible and
what not, and additionally the client is required to hold on to the
SubmittedJob if they want to do anything more with it.

We also might want to use exceptions instead of returning null in the
API, might make things clearer.

Thoughts?

Robert


>
>
> The idea is to deprecate (and sometime in the future - timing tbd -
> remove) those methods of the sling event api which we either
> consider
> expensive to support (eg maintaining large indexes for queries) or
> making assumptions/limitations on the underlying implementations and
> thus prohibit usable alternative implementations such as messaging-
> based
> ones.
>
>
> Now to the discussion as to what exactly we want to deprecate and how
> we
> see alternatives (or not) for each (all methods of JobManager [2]):
>
> (A) addJob, createJob: Those must stay
>
> (B) getQueue(s), get(Topic)Statistics: Those can probably stay
>
> (C) getJob, findJobs (and QueueType): those are the main query
> methods
> that should be deprecated. We'd not provide an alternative, but
> could
> suggest those who want to maintain job information could do that
> themselves in a shared place.
>
> (D) getJobById, retryJobById: both assume some kind of job storage
> that
> can be accesses in an arbitrary way. Thus I'd argue both could be
> deprecated. Alternative suggestion similar to (C). Edge-case.
>
> (E) removeJobById: can imv stay. the api makes no hard requirement
> to
> check if the job ever existed, however it requires that the job be
> removed when returning true, so requires (expensive) synchronicity.
> Doable. Edge-case.
>
> (F) stopJobById: can imv stay.
>
> (F) getScheduledJobs: while this requires some form of job storage
> it
> only affects a small amount of data. But if we'd want to make the
> job
> api messaging-ready it would also have to be deprecated. Edge-case.
>
>
> Cheers,
> Stefan
> --
> [0] - https://issues.apache.org/jira/browse/SLING-5884
> [1] - http://sling.markmail.org/message/k3hjqcvnnabsb47j
> [2] -
> https://github.com/apache/sling-org-apache-sling-event-api/blob/master/src/main/java/org/apache/sling/event/jobs/JobManager.java


Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Deprecate query part of Sling Event API

Timothee Maret-2
Hi,

This thread reminds me that Ian built Sling Jobs module [0]. IIUC from
reading the project readme, Sling Jobs design focused on supporting the
APIs from the Sling Events module that can be implemented efficiently on a
messaging system.

I am thinking that the Sling Jobs API could help defining the Sling Events
API to be deprecated or could serve as a new API (inline with Robert
suggestion).

Cheers,

Timothee


[0]
https://github.com/apache/sling-org-apache-sling-jobs#apache-sling-jobs-support

Le lun. 29 oct. 2018 à 16:53, Robert Munteanu <[hidden email]> a écrit :

> Hi Stefan,
>
> On Mon, 2018-10-29 at 14:47 +0100, Stefan Egli wrote:
> > Hi,
> >
> > I'd like to reactivate SLING-5884 and a related discussion [1] we
> > had
> > about deprecating the query part of the Sling Event API.
> >
> > TL;DR: Open question is exactly which of the JobManager methods do
> > we
> > want to deprecate.
>
>
> How about depreacting the JobManager completely and creating new API
> which does exactly what we want?
>
> We can switch the JobManagerImpl to being a component registered only
> if a certain configuration property is set. We can set the default to
> 'false' and log a WARN when we register that component to make it clear
> that it's not going to be supported. Eventually we can stop registering
> the implementation outright and make the inconvenient methods throw an
> UnsupportedOperationException. But I would prefer to create a new API
> for new 'realities'.
>
> I know the transition will be more inconvenient, but a new API will
> also force clients to make conscious choices about what they want to
> support and a clear transition path.
>
> While not giving it a lot of thought to the names and the API methods
> we could support, here's a strawman API we can bash:
>
> ----------------
>
> // do we need both submit and create?
> interface JobManager2 {
>   SubmittedJob submitJob(...)
>   JobBuilder createJob(...)
> }
>
> // do we need both stop and remove?
> interface SubmittedJob {
>   void retryJob()
>   void removeJob()
>   void stopJob()
> }
>
> interface JobStatisticsManager {
>   getQueue(...);
>   getQueues();
>   getStatistics();
>   // etc, etc
> }
>
> ----------------
>
> The separation of concerns makes it quite clear what is possible and
> what not, and additionally the client is required to hold on to the
> SubmittedJob if they want to do anything more with it.
>
> We also might want to use exceptions instead of returning null in the
> API, might make things clearer.
>
> Thoughts?
>
> Robert
>
>
> >
> >
> > The idea is to deprecate (and sometime in the future - timing tbd -
> > remove) those methods of the sling event api which we either
> > consider
> > expensive to support (eg maintaining large indexes for queries) or
> > making assumptions/limitations on the underlying implementations and
> > thus prohibit usable alternative implementations such as messaging-
> > based
> > ones.
> >
> >
> > Now to the discussion as to what exactly we want to deprecate and how
> > we
> > see alternatives (or not) for each (all methods of JobManager [2]):
> >
> > (A) addJob, createJob: Those must stay
> >
> > (B) getQueue(s), get(Topic)Statistics: Those can probably stay
> >
> > (C) getJob, findJobs (and QueueType): those are the main query
> > methods
> > that should be deprecated. We'd not provide an alternative, but
> > could
> > suggest those who want to maintain job information could do that
> > themselves in a shared place.
> >
> > (D) getJobById, retryJobById: both assume some kind of job storage
> > that
> > can be accesses in an arbitrary way. Thus I'd argue both could be
> > deprecated. Alternative suggestion similar to (C). Edge-case.
> >
> > (E) removeJobById: can imv stay. the api makes no hard requirement
> > to
> > check if the job ever existed, however it requires that the job be
> > removed when returning true, so requires (expensive) synchronicity.
> > Doable. Edge-case.
> >
> > (F) stopJobById: can imv stay.
> >
> > (F) getScheduledJobs: while this requires some form of job storage
> > it
> > only affects a small amount of data. But if we'd want to make the
> > job
> > api messaging-ready it would also have to be deprecated. Edge-case.
> >
> >
> > Cheers,
> > Stefan
> > --
> > [0] - https://issues.apache.org/jira/browse/SLING-5884
> > [1] - http://sling.markmail.org/message/k3hjqcvnnabsb47j
> > [2] -
> >
> https://github.com/apache/sling-org-apache-sling-event-api/blob/master/src/main/java/org/apache/sling/event/jobs/JobManager.java
>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Deprecate query part of Sling Event API

Carsten Ziegeler
In reply to this post by Robert Munteanu-2
Hi,

we already have a job API 2.0 (in commons I think) - but that's another
proprietary API which knowone outside of this circle knows about.
Deprecating means reducing complexity and making eventually the
implementation easier while at the same time having at least some form
of compatibility.

Eventually we should move to some ootb solution for job processing and
not reinvent the wheel another time. (We should have never started with
our own job processing API, but that went out of the window already). So
deprecation is a path to that other thing.

Carsten

Am 29.10.2018 um 16:53 schrieb Robert Munteanu:

> Hi Stefan,
>
> On Mon, 2018-10-29 at 14:47 +0100, Stefan Egli wrote:
>> Hi,
>>
>> I'd like to reactivate SLING-5884 and a related discussion [1] we
>> had
>> about deprecating the query part of the Sling Event API.
>>
>> TL;DR: Open question is exactly which of the JobManager methods do
>> we
>> want to deprecate.
>
>
> How about depreacting the JobManager completely and creating new API
> which does exactly what we want?
>
> We can switch the JobManagerImpl to being a component registered only
> if a certain configuration property is set. We can set the default to
> 'false' and log a WARN when we register that component to make it clear
> that it's not going to be supported. Eventually we can stop registering
> the implementation outright and make the inconvenient methods throw an
> UnsupportedOperationException. But I would prefer to create a new API
> for new 'realities'.
>
> I know the transition will be more inconvenient, but a new API will
> also force clients to make conscious choices about what they want to
> support and a clear transition path.
>
> While not giving it a lot of thought to the names and the API methods
> we could support, here's a strawman API we can bash:
>
> ----------------
>
> // do we need both submit and create?
> interface JobManager2 {
>    SubmittedJob submitJob(...)
>    JobBuilder createJob(...)
> }
>
> // do we need both stop and remove?
> interface SubmittedJob {
>    void retryJob()
>    void removeJob()
>    void stopJob()
> }
>
> interface JobStatisticsManager {
>    getQueue(...);
>    getQueues();
>    getStatistics();
>    // etc, etc
> }
>
> ----------------
>
> The separation of concerns makes it quite clear what is possible and
> what not, and additionally the client is required to hold on to the
> SubmittedJob if they want to do anything more with it.
>
> We also might want to use exceptions instead of returning null in the
> API, might make things clearer.
>
> Thoughts?
>
> Robert
>
>
>>
>>
>> The idea is to deprecate (and sometime in the future - timing tbd -
>> remove) those methods of the sling event api which we either
>> consider
>> expensive to support (eg maintaining large indexes for queries) or
>> making assumptions/limitations on the underlying implementations and
>> thus prohibit usable alternative implementations such as messaging-
>> based
>> ones.
>>
>>
>> Now to the discussion as to what exactly we want to deprecate and how
>> we
>> see alternatives (or not) for each (all methods of JobManager [2]):
>>
>> (A) addJob, createJob: Those must stay
>>
>> (B) getQueue(s), get(Topic)Statistics: Those can probably stay
>>
>> (C) getJob, findJobs (and QueueType): those are the main query
>> methods
>> that should be deprecated. We'd not provide an alternative, but
>> could
>> suggest those who want to maintain job information could do that
>> themselves in a shared place.
>>
>> (D) getJobById, retryJobById: both assume some kind of job storage
>> that
>> can be accesses in an arbitrary way. Thus I'd argue both could be
>> deprecated. Alternative suggestion similar to (C). Edge-case.
>>
>> (E) removeJobById: can imv stay. the api makes no hard requirement
>> to
>> check if the job ever existed, however it requires that the job be
>> removed when returning true, so requires (expensive) synchronicity.
>> Doable. Edge-case.
>>
>> (F) stopJobById: can imv stay.
>>
>> (F) getScheduledJobs: while this requires some form of job storage
>> it
>> only affects a small amount of data. But if we'd want to make the
>> job
>> api messaging-ready it would also have to be deprecated. Edge-case.
>>
>>
>> Cheers,
>> Stefan
>> --
>> [0] - https://issues.apache.org/jira/browse/SLING-5884
>> [1] - http://sling.markmail.org/message/k3hjqcvnnabsb47j
>> [2] -
>> https://github.com/apache/sling-org-apache-sling-event-api/blob/master/src/main/java/org/apache/sling/event/jobs/JobManager.java
>
>

--
Carsten Ziegeler
Adobe Research Switzerland
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Deprecate query part of Sling Event API

Stefan Egli-2
In reply to this post by Timothee Maret-2
Right, Sling Jobs can help in defining the deprecation, which is exactly
what I've use it for now. (btw: I went even further in restricting the
API, as I think retryJobById and getJobById are potentially expensive as
well, thus lean towards deprecating those too - not sure about
getScheduledJobs...).

Re new vs deprecation: I'd say if we would see need for changing the
actual API, ie the signatures, other than plain deprecating it, that
would be an argument for a new API. So I see that as the key question.
As if we just deprecate, there are of course several advantages: those
that don't violate anything can keep their code unchanged - the
implementation remains the same for now - no need to implement
additional, probably slightly different API... - no flags needed..

Cheers,
Stefan

On 29.10.18 17:15, Timothee Maret wrote:

> Hi,
>
> This thread reminds me that Ian built Sling Jobs module [0]. IIUC from
> reading the project readme, Sling Jobs design focused on supporting the
> APIs from the Sling Events module that can be implemented efficiently on a
> messaging system.
>
> I am thinking that the Sling Jobs API could help defining the Sling Events
> API to be deprecated or could serve as a new API (inline with Robert
> suggestion).
>
> Cheers,
>
> Timothee
>
>
> [0]
> https://github.com/apache/sling-org-apache-sling-jobs#apache-sling-jobs-support
>
> Le lun. 29 oct. 2018 à 16:53, Robert Munteanu <[hidden email]> a écrit :
>
>> Hi Stefan,
>>
>> On Mon, 2018-10-29 at 14:47 +0100, Stefan Egli wrote:
>>> Hi,
>>>
>>> I'd like to reactivate SLING-5884 and a related discussion [1] we
>>> had
>>> about deprecating the query part of the Sling Event API.
>>>
>>> TL;DR: Open question is exactly which of the JobManager methods do
>>> we
>>> want to deprecate.
>>
>>
>> How about depreacting the JobManager completely and creating new API
>> which does exactly what we want?
>>
>> We can switch the JobManagerImpl to being a component registered only
>> if a certain configuration property is set. We can set the default to
>> 'false' and log a WARN when we register that component to make it clear
>> that it's not going to be supported. Eventually we can stop registering
>> the implementation outright and make the inconvenient methods throw an
>> UnsupportedOperationException. But I would prefer to create a new API
>> for new 'realities'.
>>
>> I know the transition will be more inconvenient, but a new API will
>> also force clients to make conscious choices about what they want to
>> support and a clear transition path.
>>
>> While not giving it a lot of thought to the names and the API methods
>> we could support, here's a strawman API we can bash:
>>
>> ----------------
>>
>> // do we need both submit and create?
>> interface JobManager2 {
>>    SubmittedJob submitJob(...)
>>    JobBuilder createJob(...)
>> }
>>
>> // do we need both stop and remove?
>> interface SubmittedJob {
>>    void retryJob()
>>    void removeJob()
>>    void stopJob()
>> }
>>
>> interface JobStatisticsManager {
>>    getQueue(...);
>>    getQueues();
>>    getStatistics();
>>    // etc, etc
>> }
>>
>> ----------------
>>
>> The separation of concerns makes it quite clear what is possible and
>> what not, and additionally the client is required to hold on to the
>> SubmittedJob if they want to do anything more with it.
>>
>> We also might want to use exceptions instead of returning null in the
>> API, might make things clearer.
>>
>> Thoughts?
>>
>> Robert
>>
>>
>>>
>>>
>>> The idea is to deprecate (and sometime in the future - timing tbd -
>>> remove) those methods of the sling event api which we either
>>> consider
>>> expensive to support (eg maintaining large indexes for queries) or
>>> making assumptions/limitations on the underlying implementations and
>>> thus prohibit usable alternative implementations such as messaging-
>>> based
>>> ones.
>>>
>>>
>>> Now to the discussion as to what exactly we want to deprecate and how
>>> we
>>> see alternatives (or not) for each (all methods of JobManager [2]):
>>>
>>> (A) addJob, createJob: Those must stay
>>>
>>> (B) getQueue(s), get(Topic)Statistics: Those can probably stay
>>>
>>> (C) getJob, findJobs (and QueueType): those are the main query
>>> methods
>>> that should be deprecated. We'd not provide an alternative, but
>>> could
>>> suggest those who want to maintain job information could do that
>>> themselves in a shared place.
>>>
>>> (D) getJobById, retryJobById: both assume some kind of job storage
>>> that
>>> can be accesses in an arbitrary way. Thus I'd argue both could be
>>> deprecated. Alternative suggestion similar to (C). Edge-case.
>>>
>>> (E) removeJobById: can imv stay. the api makes no hard requirement
>>> to
>>> check if the job ever existed, however it requires that the job be
>>> removed when returning true, so requires (expensive) synchronicity.
>>> Doable. Edge-case.
>>>
>>> (F) stopJobById: can imv stay.
>>>
>>> (F) getScheduledJobs: while this requires some form of job storage
>>> it
>>> only affects a small amount of data. But if we'd want to make the
>>> job
>>> api messaging-ready it would also have to be deprecated. Edge-case.
>>>
>>>
>>> Cheers,
>>> Stefan
>>> --
>>> [0] - https://issues.apache.org/jira/browse/SLING-5884
>>> [1] - http://sling.markmail.org/message/k3hjqcvnnabsb47j
>>> [2] -
>>>
>> https://github.com/apache/sling-org-apache-sling-event-api/blob/master/src/main/java/org/apache/sling/event/jobs/JobManager.java
>>
>>
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Deprecate query part of Sling Event API

Robert Munteanu-2
In reply to this post by Carsten Ziegeler
On Mon, 2018-10-29 at 17:17 +0100, Carsten Ziegeler wrote:
> Eventually we should move to some ootb solution for job processing
> and
> not reinvent the wheel another time. (We should have never started
> with
> our own job processing API, but that went out of the window already).
> So
> deprecation is a path to that other thing.

If the plan is to eventually move to a different API then it makes no
sense to invest in something else that we design, agreed.

Robert

Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Deprecate query part of Sling Event API

Timothee Maret-2
In reply to this post by Stefan Egli-2
Hi,

Le lun. 29 oct. 2018 à 17:42, Stefan Egli <[hidden email]> a écrit :

> Right, Sling Jobs can help in defining the deprecation, which is exactly
> what I've use it for now. (btw: I went even further in restricting the
> API, as I think retryJobById and getJobById are potentially expensive as
> well, thus lean towards deprecating those too


+1 with deprecating retryJobById and getJobById. Those signatures hint that
the implementation will require random access to jobs which I expect to be
inefficiently supported on top of a messaging infrastructure.


> - not sure about
> getScheduledJobs...).
>

I think it makes sense to deprecate it for the reasons you shared in your
initial message.


>
> Re new vs deprecation: I'd say if we would see need for changing the
> actual API, ie the signatures, other than plain deprecating it, that
> would be an argument for a new API. So I see that as the key question.
> As if we just deprecate, there are of course several advantages: those
> that don't violate anything can keep their code unchanged - the
> implementation remains the same for now - no need to implement
> additional, probably slightly different API... - no flags needed..
>

I am also in favour or deprecating without replacement, without starting a
new API.

Cheers,

Timothee


>
> Cheers,
> Stefan
>
> On 29.10.18 17:15, Timothee Maret wrote:
> > Hi,
> >
> > This thread reminds me that Ian built Sling Jobs module [0]. IIUC from
> > reading the project readme, Sling Jobs design focused on supporting the
> > APIs from the Sling Events module that can be implemented efficiently on
> a
> > messaging system.
> >
> > I am thinking that the Sling Jobs API could help defining the Sling
> Events
> > API to be deprecated or could serve as a new API (inline with Robert
> > suggestion).
> >
> > Cheers,
> >
> > Timothee
> >
> >
> > [0]
> >
> https://github.com/apache/sling-org-apache-sling-jobs#apache-sling-jobs-support
> >
> > Le lun. 29 oct. 2018 à 16:53, Robert Munteanu <[hidden email]> a
> écrit :
> >
> >> Hi Stefan,
> >>
> >> On Mon, 2018-10-29 at 14:47 +0100, Stefan Egli wrote:
> >>> Hi,
> >>>
> >>> I'd like to reactivate SLING-5884 and a related discussion [1] we
> >>> had
> >>> about deprecating the query part of the Sling Event API.
> >>>
> >>> TL;DR: Open question is exactly which of the JobManager methods do
> >>> we
> >>> want to deprecate.
> >>
> >>
> >> How about depreacting the JobManager completely and creating new API
> >> which does exactly what we want?
> >>
> >> We can switch the JobManagerImpl to being a component registered only
> >> if a certain configuration property is set. We can set the default to
> >> 'false' and log a WARN when we register that component to make it clear
> >> that it's not going to be supported. Eventually we can stop registering
> >> the implementation outright and make the inconvenient methods throw an
> >> UnsupportedOperationException. But I would prefer to create a new API
> >> for new 'realities'.
> >>
> >> I know the transition will be more inconvenient, but a new API will
> >> also force clients to make conscious choices about what they want to
> >> support and a clear transition path.
> >>
> >> While not giving it a lot of thought to the names and the API methods
> >> we could support, here's a strawman API we can bash:
> >>
> >> ----------------
> >>
> >> // do we need both submit and create?
> >> interface JobManager2 {
> >>    SubmittedJob submitJob(...)
> >>    JobBuilder createJob(...)
> >> }
> >>
> >> // do we need both stop and remove?
> >> interface SubmittedJob {
> >>    void retryJob()
> >>    void removeJob()
> >>    void stopJob()
> >> }
> >>
> >> interface JobStatisticsManager {
> >>    getQueue(...);
> >>    getQueues();
> >>    getStatistics();
> >>    // etc, etc
> >> }
> >>
> >> ----------------
> >>
> >> The separation of concerns makes it quite clear what is possible and
> >> what not, and additionally the client is required to hold on to the
> >> SubmittedJob if they want to do anything more with it.
> >>
> >> We also might want to use exceptions instead of returning null in the
> >> API, might make things clearer.
> >>
> >> Thoughts?
> >>
> >> Robert
> >>
> >>
> >>>
> >>>
> >>> The idea is to deprecate (and sometime in the future - timing tbd -
> >>> remove) those methods of the sling event api which we either
> >>> consider
> >>> expensive to support (eg maintaining large indexes for queries) or
> >>> making assumptions/limitations on the underlying implementations and
> >>> thus prohibit usable alternative implementations such as messaging-
> >>> based
> >>> ones.
> >>>
> >>>
> >>> Now to the discussion as to what exactly we want to deprecate and how
> >>> we
> >>> see alternatives (or not) for each (all methods of JobManager [2]):
> >>>
> >>> (A) addJob, createJob: Those must stay
> >>>
> >>> (B) getQueue(s), get(Topic)Statistics: Those can probably stay
> >>>
> >>> (C) getJob, findJobs (and QueueType): those are the main query
> >>> methods
> >>> that should be deprecated. We'd not provide an alternative, but
> >>> could
> >>> suggest those who want to maintain job information could do that
> >>> themselves in a shared place.
> >>>
> >>> (D) getJobById, retryJobById: both assume some kind of job storage
> >>> that
> >>> can be accesses in an arbitrary way. Thus I'd argue both could be
> >>> deprecated. Alternative suggestion similar to (C). Edge-case.
> >>>
> >>> (E) removeJobById: can imv stay. the api makes no hard requirement
> >>> to
> >>> check if the job ever existed, however it requires that the job be
> >>> removed when returning true, so requires (expensive) synchronicity.
> >>> Doable. Edge-case.
> >>>
> >>> (F) stopJobById: can imv stay.
> >>>
> >>> (F) getScheduledJobs: while this requires some form of job storage
> >>> it
> >>> only affects a small amount of data. But if we'd want to make the
> >>> job
> >>> api messaging-ready it would also have to be deprecated. Edge-case.
> >>>
> >>>
> >>> Cheers,
> >>> Stefan
> >>> --
> >>> [0] - https://issues.apache.org/jira/browse/SLING-5884
> >>> [1] - http://sling.markmail.org/message/k3hjqcvnnabsb47j
> >>> [2] -
> >>>
> >>
> https://github.com/apache/sling-org-apache-sling-event-api/blob/master/src/main/java/org/apache/sling/event/jobs/JobManager.java
> >>
> >>
> >>
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Deprecate query part of Sling Event API

Robert Munteanu-2
In reply to this post by Stefan Egli-2
On Mon, 2018-10-29 at 17:42 +0100, Stefan Egli wrote:
> Right, Sling Jobs can help in defining the deprecation, which is
> exactly
> what I've use it for now. (btw: I went even further in restricting
> the
> API, as I think retryJobById and getJobById are potentially expensive
> as
> well, thus lean towards deprecating those too - not sure about
> getScheduledJobs...).

Those don't have to be expensive if you hold the state 'client-side'
with the SubmittedJob object, then you simply submit the same data. (I
may miss some specific knowledge of how eventing works though).

>
> Re new vs deprecation: I'd say if we would see need for changing the
> actual API, ie the signatures, other than plain deprecating it, that
> would be an argument for a new API. So I see that as the key
> question.
> As if we just deprecate, there are of course several advantages:
> those
> that don't violate anything can keep their code unchanged - the
> implementation remains the same for now - no need to implement
> additional, probably slightly different API... - no flags needed..

Yes, the discussion of deprecating vs adding new API is ... interesting
:-) My personal opinion is that an interface with half of its methods
deprecated is confusing and should not be used. But again, that is
something up for discussion and just a personal opinion.

Additonally,if the end-goal is to eventually replace it with a well-
known external API then there is no point in investing in yet another
API that is going to be replaced in a couple of years.

Thanks,

Robert

Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Deprecate query part of Sling Event API

Bertrand Delacretaz
Hi,

On Tue, Oct 30, 2018 at 10:08 AM Robert Munteanu <[hidden email]> wrote:
>...My personal opinion is that an interface with half of its methods
> deprecated is confusing and should not be used....

I agree, and we might use the following trick to avoid that with
minimal disruption:

-Consider the current interface "Legacy" that needs half of its
methods deprecated
-Move the non-deprecated methods to a new "Modern" interface
-Legacy inherits from Modern

Existing clients can then use Legacy with no change.
New clients move to Modern, to make it clear that they don't want to
use legacy stuff.
Mark the whole Legacy interface as deprecated, recommend moving to Modern
Existing services are explicitly registered as both Modern and Legacy.

No need to redesign things, just move them around.

WDYT?

-Bertrand
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Deprecate query part of Sling Event API

Robert Munteanu-2
On Tue, 2018-10-30 at 10:22 +0100, Bertrand Delacretaz wrote:

> Hi,
>
> On Tue, Oct 30, 2018 at 10:08 AM Robert Munteanu <[hidden email]>
> wrote:
> > ...My personal opinion is that an interface with half of its
> > methods
> > deprecated is confusing and should not be used....
>
> I agree, and we might use the following trick to avoid that with
> minimal disruption:
>
> -Consider the current interface "Legacy" that needs half of its
> methods deprecated
> -Move the non-deprecated methods to a new "Modern" interface
> -Legacy inherits from Modern
>
> Existing clients can then use Legacy with no change.
> New clients move to Modern, to make it clear that they don't want to
> use legacy stuff.
> Mark the whole Legacy interface as deprecated, recommend moving to
> Modern
> Existing services are explicitly registered as both Modern and
> Legacy.
>
> No need to redesign things, just move them around.
>
> WDYT?

+1, sounds like a good approach to me.

Robert

Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Deprecate query part of Sling Event API

Stefan Egli-2
In reply to this post by Robert Munteanu-2
On 30.10.18 10:08, Robert Munteanu wrote:

> On Mon, 2018-10-29 at 17:42 +0100, Stefan Egli wrote:
>> (btw: I went even further in restricting the
>> API, as I think retryJobById and getJobById are potentially expensive
>> as
>> well, thus lean towards deprecating those too - not sure about
>> getScheduledJobs...).
>
> Those don't have to be expensive if you hold the state 'client-side'
> with the SubmittedJob object, then you simply submit the same data. (I
> may miss some specific knowledge of how eventing works though).

That would imply changing the API - then it might be done cheaper
agreed. But with the existing API all there is is the jobId (a String),
in which case you need to look up the job data somewhere, for both
getJobById and retryJobById (but not for stopJobById and removeJobById
as they don't return the Job).

Cheers,
Stefan
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Deprecate query part of Sling Event API

Stefan Egli-2
In reply to this post by Bertrand Delacretaz
On 30.10.18 10:22, Bertrand Delacretaz wrote:

> Hi,
>
> I agree, and we might use the following trick to avoid that with
> minimal disruption:
>
> -Consider the current interface "Legacy" that needs half of its
> methods deprecated
> -Move the non-deprecated methods to a new "Modern" interface
> -Legacy inherits from Modern
>
> Existing clients can then use Legacy with no change.
> New clients move to Modern, to make it clear that they don't want to
> use legacy stuff.
> Mark the whole Legacy interface as deprecated, recommend moving to Modern
> Existing services are explicitly registered as both Modern and Legacy.
>
> No need to redesign things, just move them around.
>
> WDYT?

+1, which would bring us to naming.. one suggestion would be

/** @deprecated */
public interface JobManager extends JobManager2 /* Modern */ {
..
}

Cheers,
Stefan
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Deprecate query part of Sling Event API

Roy Teeuwen
Hey guys,

Seeing as you are all talking about deprecating or even removing the entire Sling Job API in the future, what would you propose as alternative / migration path? We have a lot JobConsumers in our projects, what would be a reason to stop using them?

Greets,
Roy

> On 30 Oct 2018, at 11:27, Stefan Egli <[hidden email]> wrote:
>
> On 30.10.18 10:22, Bertrand Delacretaz wrote:
>> Hi,
>> I agree, and we might use the following trick to avoid that with
>> minimal disruption:
>> -Consider the current interface "Legacy" that needs half of its
>> methods deprecated
>> -Move the non-deprecated methods to a new "Modern" interface
>> -Legacy inherits from Modern
>> Existing clients can then use Legacy with no change.
>> New clients move to Modern, to make it clear that they don't want to
>> use legacy stuff.
>> Mark the whole Legacy interface as deprecated, recommend moving to Modern
>> Existing services are explicitly registered as both Modern and Legacy.
>> No need to redesign things, just move them around.
>> WDYT?
>
> +1, which would bring us to naming.. one suggestion would be
>
> /** @deprecated */
> public interface JobManager extends JobManager2 /* Modern */ {
> ..
> }
>
> Cheers,
> Stefan

Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Deprecate query part of Sling Event API

Stefan Egli-2
Hi Roy,

I think removal of the job API hasn't been discussed much yet, so
that'll probably first have to happen separately.

Re deprecating the job queries: I think we should better understand the
use cases for doing these queries. Why is it not possible to fire a job
and forget, and why are queries needed/used. Ideally there would be an
alternative data structure (eg the data the job operates on) which would
be queried instead of the job itself.

Cheers,
Stefan

On 15.12.18 18:39, Roy Teeuwen wrote:

> Hey guys,
>
> Seeing as you are all talking about deprecating or even removing the entire Sling Job API in the future, what would you propose as alternative / migration path? We have a lot JobConsumers in our projects, what would be a reason to stop using them?
>
> Greets,
> Roy
>
>> On 30 Oct 2018, at 11:27, Stefan Egli <[hidden email]> wrote:
>>
>> On 30.10.18 10:22, Bertrand Delacretaz wrote:
>>> Hi,
>>> I agree, and we might use the following trick to avoid that with
>>> minimal disruption:
>>> -Consider the current interface "Legacy" that needs half of its
>>> methods deprecated
>>> -Move the non-deprecated methods to a new "Modern" interface
>>> -Legacy inherits from Modern
>>> Existing clients can then use Legacy with no change.
>>> New clients move to Modern, to make it clear that they don't want to
>>> use legacy stuff.
>>> Mark the whole Legacy interface as deprecated, recommend moving to Modern
>>> Existing services are explicitly registered as both Modern and Legacy.
>>> No need to redesign things, just move them around.
>>> WDYT?
>>
>> +1, which would bring us to naming.. one suggestion would be
>>
>> /** @deprecated */
>> public interface JobManager extends JobManager2 /* Modern */ {
>> ..
>> }
>>
>> Cheers,
>> Stefan
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Deprecate query part of Sling Event API

Stefan Egli-2
One more thought, which has been discussed offline earlier:

While it might take a while to agree on the deprecation and on how we
recommend them to be replaced, there is a short-term option that would
already help lower load on systems:

We could introduce a config option which would allow a job queue to
opt-out of job queries voluntarily. That would allow the implementation
to treat those jobs differently (cheaper, without indexing them).

I guess such a new opt-out of queries mechanism could be less
controversial and provide at least some short-term gain. It doesn't
answer the question how job queries should be replaced though..

Cheers,
Stefan


On 17.12.18 14:02, Stefan Egli wrote:

> Hi Roy,
>
> I think removal of the job API hasn't been discussed much yet, so
> that'll probably first have to happen separately.
>
> Re deprecating the job queries: I think we should better understand the
> use cases for doing these queries. Why is it not possible to fire a job
> and forget, and why are queries needed/used. Ideally there would be an
> alternative data structure (eg the data the job operates on) which would
> be queried instead of the job itself.
>
> Cheers,
> Stefan
>
> On 15.12.18 18:39, Roy Teeuwen wrote:
>> Hey guys,
>>
>> Seeing as you are all talking about deprecating or even removing the
>> entire Sling Job API in the future, what would you propose as
>> alternative / migration path? We have a lot JobConsumers in our
>> projects, what would be a reason to stop using them?
>>
>> Greets,
>> Roy
>>
>>> On 30 Oct 2018, at 11:27, Stefan Egli <[hidden email]> wrote:
>>>
>>> On 30.10.18 10:22, Bertrand Delacretaz wrote:
>>>> Hi,
>>>> I agree, and we might use the following trick to avoid that with
>>>> minimal disruption:
>>>> -Consider the current interface "Legacy" that needs half of its
>>>> methods deprecated
>>>> -Move the non-deprecated methods to a new "Modern" interface
>>>> -Legacy inherits from Modern
>>>> Existing clients can then use Legacy with no change.
>>>> New clients move to Modern, to make it clear that they don't want to
>>>> use legacy stuff.
>>>> Mark the whole Legacy interface as deprecated, recommend moving to
>>>> Modern
>>>> Existing services are explicitly registered as both Modern and Legacy.
>>>> No need to redesign things, just move them around.
>>>> WDYT?
>>>
>>> +1, which would bring us to naming.. one suggestion would be
>>>
>>> /** @deprecated */
>>> public interface JobManager extends JobManager2 /* Modern */ {
>>> ..
>>> }
>>>
>>> Cheers,
>>> Stefan
>>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Deprecate query part of Sling Event API

Stefan Egli-2
Hi,

I've given this job query issue some more thought and would like us to
discuss and hopefully soon agree on which way we go:


Variant A: deprecate job query methods (as suggested originally):
* upside: eventually we'll have a cleaner job API
* downside: users of the job query API need to find alternatives to
queries, individually


Variant B: allow disabling job queries (https://s.apache.org/68ys):
* upside: allows performance optimizations on a case-by-case basis
(optimizations include not requiring indexing), thereby promoting
query-less use cases going forward without API deprecation nevertheless.
* downside: we stick to the current job API


Note that in both cases the 'org.apache.sling.event.jobs' export version
will have to be updated from 2.0.1 to 2.1.0 - which will affect
users/customers that implement interfaces from that package (that's not
something typical though, but there are a few exotic cases where that's
the case).


At this stage I'm in favour of variant B as I don't see a good
alternative for job queries other than users re-implementing a fair
chunk of the sling event implementation themselves (which would sort of
defeat the purpose, besides the fact that it would not be trivial as
there aren't enough hooks in the API for anyone other than the
implementor to do this consistently).


Cheers,
Stefan

On 17.12.18 16:08, Stefan Egli wrote:
> We could introduce a config option which would allow a job queue to
> opt-out of job queries voluntarily. That would allow the implementation
> to treat those jobs differently (cheaper, without indexing them).
>
> I guess such a new opt-out of queries mechanism could be less
> controversial and provide at least some short-term gain. It doesn't
> answer the question how job queries should be replaced though..

Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Deprecate query part of Sling Event API

Carsten Ziegeler
Hi,

I agree, variant B sounds like the better option due to the reasons you
mention. It also provides a step by step way of moving a single type of
jobs to not use search anymore.

The only downside is that this doesn't force anyone to think about her
job usage as everything just runs as before. Only if you want to improve
you can and adjust the configurations.


Regards

Carsten


Stefan Egli wrote

> Hi,
>
> I've given this job query issue some more thought and would like us to
> discuss and hopefully soon agree on which way we go:
>
>
> Variant A: deprecate job query methods (as suggested originally):
> * upside: eventually we'll have a cleaner job API
> * downside: users of the job query API need to find alternatives to
> queries, individually
>
>
> Variant B: allow disabling job queries (https://s.apache.org/68ys):
> * upside: allows performance optimizations on a case-by-case basis
> (optimizations include not requiring indexing), thereby promoting
> query-less use cases going forward without API deprecation nevertheless.
> * downside: we stick to the current job API
>
>
> Note that in both cases the 'org.apache.sling.event.jobs' export version
> will have to be updated from 2.0.1 to 2.1.0 - which will affect
> users/customers that implement interfaces from that package (that's not
> something typical though, but there are a few exotic cases where that's
> the case).
>
>
> At this stage I'm in favour of variant B as I don't see a good
> alternative for job queries other than users re-implementing a fair
> chunk of the sling event implementation themselves (which would sort of
> defeat the purpose, besides the fact that it would not be trivial as
> there aren't enough hooks in the API for anyone other than the
> implementor to do this consistently).
>
>
> Cheers,
> Stefan
>
> On 17.12.18 16:08, Stefan Egli wrote:
>> We could introduce a config option which would allow a job queue to
>> opt-out of job queries voluntarily. That would allow the
>> implementation to treat those jobs differently (cheaper, without
>> indexing them).
>>
>> I guess such a new opt-out of queries mechanism could be less
>> controversial and provide at least some short-term gain. It doesn't
>> answer the question how job queries should be replaced though..
>
--
Carsten Ziegeler
Adobe Research Switzerland
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Deprecate query part of Sling Event API

Jason E Bailey
So I'm missing something here, from what I gather so far, the methods that are being deprecated are to prevent a performance issue but the node structure that represents the jobs is in one place isn't it? How in the world are we getting a performance issue from that?

Is there a use case to demonstrate the performance problem?

--
Jason

On Tue, Jan 8, 2019, at 11:12 AM, Carsten Ziegeler wrote:

> Hi,
>
> I agree, variant B sounds like the better option due to the reasons you
> mention. It also provides a step by step way of moving a single type of
> jobs to not use search anymore.
>
> The only downside is that this doesn't force anyone to think about her
> job usage as everything just runs as before. Only if you want to improve
> you can and adjust the configurations.
>
>
> Regards
>
> Carsten
>
>
> Stefan Egli wrote
> > Hi,
> >
> > I've given this job query issue some more thought and would like us to
> > discuss and hopefully soon agree on which way we go:
> >
> >
> > Variant A: deprecate job query methods (as suggested originally):
> > * upside: eventually we'll have a cleaner job API
> > * downside: users of the job query API need to find alternatives to
> > queries, individually
> >
> >
> > Variant B: allow disabling job queries (https://s.apache.org/68ys):
> > * upside: allows performance optimizations on a case-by-case basis
> > (optimizations include not requiring indexing), thereby promoting
> > query-less use cases going forward without API deprecation nevertheless.
> > * downside: we stick to the current job API
> >
> >
> > Note that in both cases the 'org.apache.sling.event.jobs' export version
> > will have to be updated from 2.0.1 to 2.1.0 - which will affect
> > users/customers that implement interfaces from that package (that's not
> > something typical though, but there are a few exotic cases where that's
> > the case).
> >
> >
> > At this stage I'm in favour of variant B as I don't see a good
> > alternative for job queries other than users re-implementing a fair
> > chunk of the sling event implementation themselves (which would sort of
> > defeat the purpose, besides the fact that it would not be trivial as
> > there aren't enough hooks in the API for anyone other than the
> > implementor to do this consistently).
> >
> >
> > Cheers,
> > Stefan
> >
> > On 17.12.18 16:08, Stefan Egli wrote:
> >> We could introduce a config option which would allow a job queue to
> >> opt-out of job queries voluntarily. That would allow the
> >> implementation to treat those jobs differently (cheaper, without
> >> indexing them).
> >>
> >> I guess such a new opt-out of queries mechanism could be less
> >> controversial and provide at least some short-term gain. It doesn't
> >> answer the question how job queries should be replaced though..
> >
> --
> Carsten Ziegeler
> Adobe Research Switzerland
> [hidden email]
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Deprecate query part of Sling Event API

Daniel Klco-2
Yeah I must admit I'm confused as well, looking through the email threads
the original proposal is to return and a iterator rather than a collection.
If the problem is that too many jobs being returned causes heap issues, I
would think this would resolve that problem. Is there some other problem
here? If it is in query performance, could it possibly be solved using oak
indexes?

On Tue, Jan 8, 2019, 3:50 PM Jason E Bailey <[hidden email] wrote:

> So I'm missing something here, from what I gather so far, the methods that
> are being deprecated are to prevent a performance issue but the node
> structure that represents the jobs is in one place isn't it? How in the
> world are we getting a performance issue from that?
>
> Is there a use case to demonstrate the performance problem?
>
> --
> Jason
>
> On Tue, Jan 8, 2019, at 11:12 AM, Carsten Ziegeler wrote:
> > Hi,
> >
> > I agree, variant B sounds like the better option due to the reasons you
> > mention. It also provides a step by step way of moving a single type of
> > jobs to not use search anymore.
> >
> > The only downside is that this doesn't force anyone to think about her
> > job usage as everything just runs as before. Only if you want to improve
> > you can and adjust the configurations.
> >
> >
> > Regards
> >
> > Carsten
> >
> >
> > Stefan Egli wrote
> > > Hi,
> > >
> > > I've given this job query issue some more thought and would like us to
> > > discuss and hopefully soon agree on which way we go:
> > >
> > >
> > > Variant A: deprecate job query methods (as suggested originally):
> > > * upside: eventually we'll have a cleaner job API
> > > * downside: users of the job query API need to find alternatives to
> > > queries, individually
> > >
> > >
> > > Variant B: allow disabling job queries (https://s.apache.org/68ys):
> > > * upside: allows performance optimizations on a case-by-case basis
> > > (optimizations include not requiring indexing), thereby promoting
> > > query-less use cases going forward without API deprecation
> nevertheless.
> > > * downside: we stick to the current job API
> > >
> > >
> > > Note that in both cases the 'org.apache.sling.event.jobs' export
> version
> > > will have to be updated from 2.0.1 to 2.1.0 - which will affect
> > > users/customers that implement interfaces from that package (that's
> not
> > > something typical though, but there are a few exotic cases where
> that's
> > > the case).
> > >
> > >
> > > At this stage I'm in favour of variant B as I don't see a good
> > > alternative for job queries other than users re-implementing a fair
> > > chunk of the sling event implementation themselves (which would sort
> of
> > > defeat the purpose, besides the fact that it would not be trivial as
> > > there aren't enough hooks in the API for anyone other than the
> > > implementor to do this consistently).
> > >
> > >
> > > Cheers,
> > > Stefan
> > >
> > > On 17.12.18 16:08, Stefan Egli wrote:
> > >> We could introduce a config option which would allow a job queue to
> > >> opt-out of job queries voluntarily. That would allow the
> > >> implementation to treat those jobs differently (cheaper, without
> > >> indexing them).
> > >>
> > >> I guess such a new opt-out of queries mechanism could be less
> > >> controversial and provide at least some short-term gain. It doesn't
> > >> answer the question how job queries should be replaced though..
> > >
> > --
> > Carsten Ziegeler
> > Adobe Research Switzerland
> > [hidden email]
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Deprecate query part of Sling Event API

Stefan Egli-2
In reply to this post by Stefan Egli-2
On 08.01.19 17:12, Carsten Ziegeler wrote:
> I agree, variant B sounds like the better option ...
>
> The only downside is that this doesn't force anyone to think about her
> job usage as everything just runs as before. Only if you want to improve
> you can and adjust the configurations.

That's a problem yes. Only A would allow to use some force.

Perhaps we can add something to the system console to visualize which
job queues have queries enabled and which of those have queries actually
executed (since last restart). Together with a note that job queries are
now considered bad/suboptimal practice...

Cheers,
Stefan
12