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

Jump to bottom

Undefined waves are consumed before all handlers (running into JTP and JAT) are processed #137

Closed
Rizen59 opened this issue Oct 29, 2014 · 7 comments
Closed

Undefined waves are consumed before all handlers (running into JTP and JAT) are processed #137

Rizen59 opened this issue Oct 29, 2014 · 7 comments
Assignees
@sbordes
Labels
bug
Milestone
8.0.1

Comments

@Rizen59
Copy link
Contributor

Rizen59 commented Oct 29, 2014

Saw in the processUndefinedWave() method from NotifierBase class:

LOGGER.info(NOTIFIER_CONSUMES, wave.toString());
wave.status(Status.Consumed);

The wave is notified as consumed after all handlers are executed. But it not takes the execution thread into consideration. It means all WaveHandler running into JIT (where the wave is processed) will be executed before the wave status is set to "Consumed" whereas others handlers could be executed after.

Here is a concrete example:
Suppose we have 3 wave handlers (called A - B - C) associated to a WaveItem. A is configured to run into JTP, B into JIT and C into JAT.

  • The wave status is currently "Processed"
  • A will be launched into JTP but not still executed
  • B will be launch and executed
  • C will be launch into JATbut not still executed
  • The wave status is set to "Consumed"
  • B et C will be executed

Until now my interpretation of the status consumed was: "The wave has permit to execute a method and this method has terminated". If it's this case there is maybe a bug into the framework.
But it's maybe a mistake and the real interpretation could be : "The wave has permit to execute a method. It works like a messenger and do not care if the method has finished".

I'm hesitating since I've seen theses two sentences in the official documentation:

A Wave is a temporary object created to carry data througt CS-MVC components.
5. Consumed : The wave task is achieved and is marked as consumed

Regards,
@sbordes
Copy link
Member

sbordes commented Oct 29, 2014

Thanks (again) for the report, which version do you use ?

So to sum up, the trouble is to set the status Consumed to a Wave being handled by the first handler available, whereas other haven't been executed yet.

@Rizen59
Copy link
Contributor Author

Rizen59 commented Oct 29, 2014

The 7.7.0. Has it been fixed in a more recent version ?

We can not upgrade to 7.7.5 for now cause of this issue (#126)

@sbordes
Copy link
Member

sbordes commented Oct 29, 2014

The #126 issue could be backported to a future 7.7.6 that will integrate a fix for current issue.

Can you confirm that all handlers (A,B,C) are correctly executed, the annoying thing is just the bad Wave Status

@sbordes sbordes added the bug label Oct 29, 2014
@sbordes sbordes added this to the 7.7.6 milestone Oct 29, 2014
@sbordes sbordes self-assigned this Oct 29, 2014
@Rizen59
Copy link
Contributor Author

Rizen59 commented Oct 29, 2014

Yes all handlers are correctly executed.
I was listening the wave status to be notified when all handlers have finished (with one executed into JTP). This is how I've seen this, the status was changed before my method was executed.

But I could not say if it was a bug or a normal thing because of my two interpretations I told in the first post.

@sbordes
Copy link
Member

sbordes commented Oct 29, 2014

I will consider it as a bug because the wave status should take on consideration all possible handler.
I will probably compute an handler plan to know when all are done.

So I will create a maintenance 7.7.6 version, with backport of #126, do you need something else to migrate to 7.7.6.

[The 8.0.0 is ready but I'm performing a lot of refactoring before freezing the API.]

@Rizen59
Copy link
Contributor Author

Rizen59 commented Oct 29, 2014

It's not essential to me to include it as quick as you can (but for others developpers it will be a good thing of course :)). We will not probably upgrade to 7.7.6 soon due to others projects we have. Maybe in few weeks.

As I remember no others issues are blocker for this.

Thanks a lot for this quick fix and your nice job !

@sbordes sbordes modified the milestones: 8.0.1, 7.7.6 Feb 28, 2015
sbordes added a commit that referenced this issue Feb 28, 2015
@sbordes
Fix #137 Add Status.Handled in addition of Consumed
a094b16
@sbordes
Copy link
Member

sbordes commented Feb 28, 2015

This bug is fixed into 8.0.1-SNAPSHOT, do you need a patch for 7.7.6 version ?

The Wave .Status Handled has been added to manage your need, the Wave.Status.Consumed has been refactored to be triggered when the handling process starts

@sbordes sbordes closed this as completed Feb 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sbordes sbordes

Labels
bug
Projects
None yet
Milestone
8.0.1
Development

No branches or pull requests
2 participants
@Rizen59 @sbordes