Mark messages as received-by-gateway so they are not included in the next response

Question

Mark messages as received-by-gateway so they are not included in the next response

estellecomment opened this issue 8 years ago · 20 comments

From medic/cht-core#3073 :
Modify medic-gateway to call medic-api and change the status of received messages to received-by-gateway after they have been successfully written to the database. This will stop them being sent again next query and reduce the bandwidth requirements.

This will avoid step 4 in the current protocol

To do it, have gateway set the needs forwarding flag to true when storing a new message in DB.

Answer 1 · 2017-04-28T05:29:58.000Z

@estellecomment It look like you've done most of the work for this. Is it in progress, or do you want me to finish it off?

Answer 2 · 2017-04-28T08:57:58.000Z

I haven't given up on it yet :)
I unassigned myself because I'm doing something else for now, and I don't know how much of a hurry you're in. If it's urgent you can finish it, otherwise I will today/tomorrow.

Answer 3 · 2017-05-05T08:56:45.000Z

@alxndrsn Can you review?

Answer 4 · 2017-05-06T05:11:16.000Z

PR test failures are things like this:

medic.gateway.alert.SettingsDialogActivityTest > generic_shouldDisplayCorrectFields[test(AVD) - 5.0.2] FAILED 

	java.lang.RuntimeException: Waited for the root of the view hierarchy to have window focus and not be requesting layout for over 10 seconds. If you specified a non default root matcher, it may be picking a root that never takes focus. Otherwise, something is seriously wrong. Selected Root:

The espresso tests intermittently fail on Travis with errors like this. I'll re-run and see if they pass the second time.

Answer 5 · 2017-06-06T21:48:42.000Z

@alxndrsn What's the status of this? Did it fail AT?

Answer 6 · 2017-06-07T08:07:29.000Z

nah, I merged it but @alxndrsn hadn't reviewed it so reverted it.
It got split into two PRs :

code : #107 is breaking, I have to figure out why
unrelated tests : #106 is supposed to be all tests but apparently it's not, so gotta sort that out.

I have higher priority stuff that keeps happening, but it's still on my list.

Answer 7 · 2017-07-10T23:08:02.000Z

@alxndrsn Would you mind finishing this off while @estellecomment is away?

Answer 8 · 2017-08-03T17:17:07.000Z

I didn't have time to refamiliarise myself with the original ticket, and the information about motivation here seems sparse. I would like to fully consider whether this is useful, in line with discussions about the protocol and recent work on the server-side by @SCdF. But if we do want to achieve this, PR at #113

Answer 9 · 2017-08-21T04:10:14.000Z

@estellecomment Should we finish this or close it? It's been hanging around for ages...

Answer 10 · 2017-09-05T22:16:57.000Z

Ok... This isn't getting done. Kicking it out of the milestone.

Answer 11 · 2018-05-23T16:21:55.000Z

Hi @dianabarsan,

This ticket has not been touched in 90 days. Is it still relevant?

Please also ensure this ticket has a Priority, Status and Type label.(See triaging old issues for more detail)

Answer 12 · 2018-06-12T06:14:16.000Z

@alxndrsn what do you think? Worth reviving?

Answer 13 · 2019-01-21T01:14:31.000Z

This looks like it's mostly done in these PRs: #107 & #113 . Scheduling for 3.5.0 to align with the other gateway work.

Answer 14 · 2019-12-02T22:40:17.000Z

Not yet started - delaying to 3.9.0

Answer 15 · 2020-04-28T23:26:25.000Z

Delaying as this is a performance improvement only.

Answer 16 · 2020-08-06T09:44:29.000Z

Moving to AT, PR: #160
This branch is based on the branch that solves issue: #139
Please review and get #139 merged first before AT this.

Answer 17 · 2020-09-16T10:23:53.000Z

Summary

The issue is when API send to Gateway a message (sms), this message was being saved as "not needing forwarding", so its status wasn't updated as "received by gateway" and API might try to send it again to Gateway.

The fix is:
The SMS status should be set as Needs Forwarding when updating sms statuses or when saving new SMS to Gateway DB. Then Gateway will notify the status change to API from the beginning, reducing the chance of API to resend the same SMS to Gateway.

It's important to note that API <-> Gateway communicates in intervals of time, every 2 or 5 minutes.

Answer 18 · 2020-09-28T16:34:29.000Z

@latin-panda I think this is ready to go.

I've pulled the sqlite database and see the message is marked with needs_forwarding of 1. Once the attempt to send and it eventually fails. The needs_forwarding is marked as 0. Looking at the status_history on the message in couchdb. I see the proper flow through. As shown below. Also, have ensured on my phone with service that the messages are being sent properly.

      "state_history": [
        {
          "state": "pending",
          "timestamp": "2020-09-28T14:28:06.231Z"
        },
        {
          "state": "forwarded-to-gateway",
          "timestamp": "2020-09-28T14:28:58.170Z"
        },
        {
          "state": "received-by-gateway",
          "timestamp": "2020-09-28T14:29:58.089Z"
        },
        {
          "state": "forwarded-by-gateway",
          "timestamp": "2020-09-28T14:29:58.089Z"
        },
        {
          "state": "failed",
          "state_details": {
            "reason": "generic; errorCode=-1"
          },
          "timestamp": "2020-09-28T14:29:58.089Z"
        }

Answer 19 · 2021-01-10T22:44:13.000Z

@latin-panda Can this issue be closed now?

Answer 20 · 2021-01-11T02:35:31.000Z

Hi @garethbowen, yes, it's in "done" status and code was merged :)