Quantcast

[aerogear-dev] Question about metricsEndpoint

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[aerogear-dev] Question about metricsEndpoint

Jose Miguel Gallas Olmedo
Hi all,

I am trying to solve some bugish behaviour that makes the status label to be "Pending..." permanently, even when the notifications have been sent.


Well, in the metricsEndpoint.js, it iterates over all variants and flag an error or increase the "servedVariants" property -> https://github.com/aerogear/aerogear-unifiedpush-server/blob/1.1.x-dev/admin-ui/app/scripts/endpoints/metricsEndpoint.js#L28-L30

​I don't fully understand what's happening in this iteration and why there's no "else" clause. The problem is caused by this I think. During some iteration the counter is not being increased hence the servedVariants never equals totalVariants and hence (again) the label does not change.​

So the current flow would be:
1. If deliveryStatus is falsy, flag an error.
2. If servedBatches = totalBatches then increase servedVariants

Can both conditions pass? I think they can although this is not expected.
Can deliveryStatus be always truthy but the servedBatches != totalBatches? I think that what's happening and as a result, it is hiding some error that the user may want to see.

I also think that, in case of deliveryStatus, the iteration should stop.

WDYT?

​Cheers,​

--

JOSE MIGUEL GALLAS OLMEDO

ASSOCIATE QE, mobile

Red Hat 

<span href="tel:+34618488633">M: +34618488633    


_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [aerogear-dev] Question about metricsEndpoint

Matthias Wessendorf
Mind sending a PR ? 

On Thu, Apr 20, 2017 at 1:31 PM, Jose Miguel Gallas Olmedo <[hidden email]> wrote:
Hi all,

I am trying to solve some bugish behaviour that makes the status label to be "Pending..." permanently, even when the notifications have been sent.


Well, in the metricsEndpoint.js, it iterates over all variants and flag an error or increase the "servedVariants" property -> https://github.com/aerogear/aerogear-unifiedpush-server/blob/1.1.x-dev/admin-ui/app/scripts/endpoints/metricsEndpoint.js#L28-L30

​I don't fully understand what's happening in this iteration and why there's no "else" clause. The problem is caused by this I think. During some iteration the counter is not being increased hence the servedVariants never equals totalVariants and hence (again) the label does not change.​

So the current flow would be:
1. If deliveryStatus is falsy, flag an error.
2. If servedBatches = totalBatches then increase servedVariants

Can both conditions pass? I think they can although this is not expected.
Can deliveryStatus be always truthy but the servedBatches != totalBatches? I think that what's happening and as a result, it is hiding some error that the user may want to see.

I also think that, in case of deliveryStatus, the iteration should stop.

WDYT?

​Cheers,​

--

JOSE MIGUEL GALLAS OLMEDO

ASSOCIATE QE, mobile

Red Hat 

<span href="tel:+34618488633">M: +34618488633    


_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev



--

_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [aerogear-dev] Question about metricsEndpoint

Jose Miguel Gallas Olmedo
​But I am asking for a clarification of this code, I cannot send a fix if I don't understand what this does.​

On 20 April 2017 at 14:29, Matthias Wessendorf <[hidden email]> wrote:
Mind sending a PR ? 

On Thu, Apr 20, 2017 at 1:31 PM, Jose Miguel Gallas Olmedo <[hidden email]> wrote:
Hi all,

I am trying to solve some bugish behaviour that makes the status label to be "Pending..." permanently, even when the notifications have been sent.


Well, in the metricsEndpoint.js, it iterates over all variants and flag an error or increase the "servedVariants" property -> https://github.com/aerogear/aerogear-unifiedpush-server/blob/1.1.x-dev/admin-ui/app/scripts/endpoints/metricsEndpoint.js#L28-L30

​I don't fully understand what's happening in this iteration and why there's no "else" clause. The problem is caused by this I think. During some iteration the counter is not being increased hence the servedVariants never equals totalVariants and hence (again) the label does not change.​

So the current flow would be:
1. If deliveryStatus is falsy, flag an error.
2. If servedBatches = totalBatches then increase servedVariants

Can both conditions pass? I think they can although this is not expected.
Can deliveryStatus be always truthy but the servedBatches != totalBatches? I think that what's happening and as a result, it is hiding some error that the user may want to see.

I also think that, in case of deliveryStatus, the iteration should stop.

WDYT?

​Cheers,​

--

JOSE MIGUEL GALLAS OLMEDO

ASSOCIATE QE, mobile

Red Hat 

<span href="tel:+34618488633">M: +34618488633    


_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev



--

_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev



--

JOSE MIGUEL GALLAS OLMEDO

ASSOCIATE QE, mobile

Red Hat 

<span href="tel:+34618488633">M: +34618488633    


_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [aerogear-dev] Question about metricsEndpoint

Matthias Wessendorf
In reply to this post by Jose Miguel Gallas Olmedo


On Thu, Apr 20, 2017 at 1:31 PM, Jose Miguel Gallas Olmedo <[hidden email]> wrote:
Hi all,

I am trying to solve some bugish behaviour that makes the status label to be "Pending..." permanently, even when the notifications have been sent.


Well, in the metricsEndpoint.js, it iterates over all variants and flag an error or increase the "servedVariants" property -> https://github.com/aerogear/aerogear-unifiedpush-server/blob/1.1.x-dev/admin-ui/app/scripts/endpoints/metricsEndpoint.js#L28-L30

​I don't fully understand what's happening in this iteration and why there's no "else" clause. The problem is caused by this I think. During some iteration the counter is not being increased hence the servedVariants never equals totalVariants and hence (again) the label does not change.​

So the current flow would be:
1. If deliveryStatus is falsy, flag an error.
2. If servedBatches = totalBatches then increase servedVariants

Can both conditions pass? I think they can although this is not expected.

with two conditions? 
 
Can deliveryStatus be always truthy but the servedBatches != totalBatches?

Yes, each batch uses a Sender, which signals if it could submit something to the provider:

now, when we have 10 batches, starting in batch one we do set the status to true, and we continue to do so (if success) on all other coming batches.


On the server I noticed a timing issue, with the internal processing of the metrics payload via JMS


 
I think that what's happening and as a result, it is hiding some error that the user may want to see.

I also think that, in case of deliveryStatus, the iteration should stop.

WDYT?

​Cheers,​

--

JOSE MIGUEL GALLAS OLMEDO

ASSOCIATE QE, mobile

Red Hat 

<span href="tel:+34618488633">M: +34618488633    


_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev



--

_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [aerogear-dev] Question about metricsEndpoint

Jose Miguel Gallas Olmedo
Which 2 conditions?
 
The 2 conditions in the loop:

          angular.forEach(metric.variantInformations, function (variantMetric) {
            metric.totalReceivers += variantMetric.receivers;
            if (
​​
!variantMetric.deliveryStatus
) {

              metric.deliveryFailed = true;
            }
            if (
​​
variantMetric.servedBatches === variantMetric.totalBatches
) {

              metric.servedVariants += 1;
            }
          });

If there's a failure and deliveryFailed is set to true, there should be no need of increasing servedVariants counter, I think this is not robust code.


Then we should keep track of "unservedVariants" so we know when all variants have been processed (not served). Because if 1 variant has not been served and deliveryStatus is falsy, the label in the Activity Log will be permanently "Pending".

I'm not really suggesting anything in particular, just trying to understand this code. So please correct me if I'm wrong.

On 20 April 2017 at 17:56, Matthias Wessendorf <[hidden email]> wrote:


On Thu, Apr 20, 2017 at 1:31 PM, Jose Miguel Gallas Olmedo <[hidden email]> wrote:
Hi all,

I am trying to solve some bugish behaviour that makes the status label to be "Pending..." permanently, even when the notifications have been sent.


Well, in the metricsEndpoint.js, it iterates over all variants and flag an error or increase the "servedVariants" property -> https://github.com/aerogear/aerogear-unifiedpush-server/blob/1.1.x-dev/admin-ui/app/scripts/endpoints/metricsEndpoint.js#L28-L30

​I don't fully understand what's happening in this iteration and why there's no "else" clause. The problem is caused by this I think. During some iteration the counter is not being increased hence the servedVariants never equals totalVariants and hence (again) the label does not change.​

So the current flow would be:
1. If deliveryStatus is falsy, flag an error.
2. If servedBatches = totalBatches then increase servedVariants

Can both conditions pass? I think they can although this is not expected.

with two conditions? 
 
Can deliveryStatus be always truthy but the servedBatches != totalBatches?

Yes, each batch uses a Sender, which signals if it could submit something to the provider:

now, when we have 10 batches, starting in batch one we do set the status to true, and we continue to do so (if success) on all other coming batches.


On the server I noticed a timing issue, with the internal processing of the metrics payload via JMS


 
I think that what's happening and as a result, it is hiding some error that the user may want to see.

I also think that, in case of deliveryStatus, the iteration should stop.

WDYT?

​Cheers,​

--

JOSE MIGUEL GALLAS OLMEDO

ASSOCIATE QE, mobile

Red Hat 

<span href="tel:+34618488633">M: +34618488633    


_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev



--

_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev



--

JOSE MIGUEL GALLAS OLMEDO

ASSOCIATE QE, mobile

Red Hat 

<span href="tel:+34618488633">M: +34618488633    


_______________________________________________
aerogear-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/aerogear-dev
Loading...