IUSCA / SQAN

Scalable Quality Assurance for Neuroimaging - (SQAN): A full-stack system solution for extracting, translating, and logging (ETL) + web portal-based quality-control verification of DICOM-formatted medical imaging data/metadata.
https://sqan.sca.iu.edu/
Other
8 stars 3 forks source link

Real-time Updates #123

Closed youngmd closed 3 years ago

youngmd commented 4 years ago

In earlier versions of SQAN we had a websocket implementation that updated the exams and series pages to reflect new ingestion and QC events. This feature became difficult to maintain and was removed.

I have begun exploring alternatives for re-implementing this feature and have started with Server-Side Events (SSE) which offer several advantages over websockets: less overhead, clearer syntax, built-in browser support. The downside of SSE is that it is one-way (server->client) which is not an issue for use case.

To this end I have developed prototype, proof-of-concept code to understand how we will implement SSE within the SQAN api and ui. This ticket will serve as documentation of the details and reasoning of implementation

agopu commented 4 years ago

Thanks for taking the initiative with this, @youngmd - I look forward to learning more.

youngmd commented 4 years ago

For my simple test case I utilized the express-sse library and set up the following:

var SSE = require('express-sse');
var sse = new SSE(["array", "containing", "initial", "content", "(optional)"]);

router.get('/', function(req, res, next) {
  console.log('Generating new event');
  sse.send(['test','string'], 'message');
  res.json({'msg': 'ok'});
});

router.get('/stream', sse.init);

module.exports = router;

When a client connects to the /stream route they are subscribed (and will automatically reconnect), and another client accessing the '/' route generates an event and sends a message to the client. It doesn't have to be another client initiating the event, but that was the simplest way to setup events on-demand for testing.

The simplest way to test this SSE implementation is with curl:

curl https://mike.sca.iu.edu/sseapi/stream

What I found was that no events were being received by the client, until I terminated the API instance, when all events were received simultaneously. This led me to suspect that nginx was not releasing the events, and I found this after some searching: https://serverfault.com/questions/801628/for-server-sent-events-sse-what-nginx-proxy-configuration-is-appropriate

Based on that I modified the API location in the nginx config by adding following:

   location /sseapi/ {
        proxy_http_version 1.1;
        proxy_set_header Connection "";
        proxy_buffering off;
        proxy_pass http://localhost:12812/;
    }

Disabling nginx buffering allowed the events to be successfully transmitted to the client. I'm not sure this is the correct way to configure nginx (@rperigo ), but it is a starting point to allow for further development.

$ curl https://mike.sca.iu.edu/sseapi/stream
id: 0
data: "array"

id: 1
data: "containing"

id: 2
data: "initial"

id: 3
data: "content"

id: 4
data: "(optional)"

id: 5
event: message
data: ["test","string"]
rperigo commented 4 years ago

@youngmd I haven't toyed too much with these config variables in Nginx, but I could see straight disabling buffering to be a potential performance problem, depending on client connection speed and number of connections.

In our use case it may not matter much, but the SO link you gave also mentions setting X-Accel-Buffer: no for the SSE-specfic responses in the code as an ideal solution, since it would inform Nginx not to buffer those and let the rest be handled as normal. At least for right now the current setup won't cause any security issues afaik and would allow for testing further.

agopu commented 4 years ago

Is this still an issue with the SQAN/Vue refresh? How should we proceed? @youngmd @rperigo

youngmd commented 4 years ago

I put this on hold while trying to get SQAN to parity with production. The particular issue described here has been addressed by modifying the package used, and I submitted a pull request to the package maintainer to include that change. My change was merged into the package and the latest release includes the X-Accel-Buffer setting we need.

https://github.com/dpskvn/express-sse/pull/23

agopu commented 4 years ago

@youngmd Thanks for the update, we will stay tuned.

youngmd commented 4 years ago

Real-time updates have been Implemented for exam view ReQC. When a ReQC event is accepted by the API the UI will update itself, and when an exam has passed through QC the UI will also update. There are still some issues with timeouts and reconnects that need to be resolve but I would call it a successful proof of concept.

youngmd commented 4 years ago

A minor nginx config update appears to be required to allow SSE connections to persist longer than 60 seconds:

proxy_read_timeout 24h;

@rperigo I tested this on sqan-test and it fixed my issue (504 errors on client). Let me know if you see any problem with this configuration change. Maybe 24h is excessive?

Relevant stackoverflow: https://stackoverflow.com/questions/21630509/server-sent-events-connection-timeout-on-node-js-via-nginx

agopu commented 3 years ago

I believe this issue is stale. Review in Jan and close if appropriate.