yunikorn-web icon indicating copy to clipboard operation
yunikorn-web copied to clipboard

[YUNIKORN-2249] Add Accept-Encoding header when fetch queue application API

Open targetoee opened this issue 1 year ago • 5 comments

What is this PR for?

When the API response size is large, it takes a long time to transfer data. This PR adds a 'Accept-Encoding' request header. If the client (web side) indicates gzip encoding, the scheduler will compress the data before sending it back. So it can reduce the data size and transfer time.

What type of PR is it?

  • [ ] - Bug Fix
  • [x] - Improvement
  • [ ] - Feature
  • [ ] - Documentation
  • [ ] - Hot Fix
  • [ ] - Refactoring

Todos

N/A

What is the Jira issue?

YUNIKORN-2249

How should this be tested?

local build

Screenshots (if appropriate)

N/A

Questions:

N/A

targetoee avatar Dec 12 '23 15:12 targetoee

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (8325d7c) 66.66% compared to head (5ea6626) 66.66%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #158   +/-   ##
=======================================
  Coverage   66.66%   66.66%           
=======================================
  Files           1        1           
  Lines          30       30           
=======================================
  Hits           20       20           
  Misses          7        7           
  Partials        3        3           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Dec 12 '23 18:12 codecov-commenter

More general: should we not be setting this on all requests that we do? I can see requests also get large when we do get a large number of nodes etc. If the server does not support it or decides not to zip (i.e. below the MTU size) it should all be OK even if set.

wilfred-s avatar Dec 19 '23 06:12 wilfred-s

Most browsers automatically include 'Accept-Encoding: gzip, deflate' in the request header, so adding it is redundant. Additionally, we may not need to compress or decompress all the data on web at present. Simply providing a compression option for the user may be enough. Keep things as before is one choice. Or any other suggestion?

targetoee avatar Mar 12 '24 05:03 targetoee

Personally, gzip is good enough

chia7712 avatar Mar 12 '24 06:03 chia7712

Most browsers automatically include 'Accept-Encoding: gzip, deflate' in the request header, so adding it is redundant. Additionally, we may not need to compress or decompress all the data on web at present. Simply providing a compression option for the user may be enough. Keep things as before is one choice. Or any other suggestion?

Except not all clients are browsers (and the header is not added by default to things like API calls from javascript in many frameworks).

The purpose of the Accept-Encoding header is to allow content negotiation. If the header is not present on a request, the server MUST interpret it as if Accept-Encoding: identity were sent -- the identity encoding is effectively a no-op. Clients are required to accept unencoded responses; they are not required to accept any form of compression at all. The purpose of the header is to inform the server of the client's capabilities, so that the server may choose a compatible encoding. The server MUST choose from one of the available encodings, or fall back to identity if no common encoding exists. Since we are adding gzip support to our server, we simply need to test if the client supports it; if so we use gzip otherwise we don't compress. We MUST NOT send an error in any case, and we MUST NOT send gzip encoded data to a client which has not requested it explicitly.

By the way -- I'm not trying to be obnoxious with the MUST and MUST NOT terminology; those are taken directly from the language used by the RFCs that define how HTTP is supposed to work.

https://datatracker.ietf.org/doc/html/rfc9110#name-accept-encoding

Note that some of the tests given that RFC aren't completely practical; For example, "If no Accept-Encoding header field is in the request, any content coding is considered acceptable by the user agent" -- this isn't really ever true in practice. If I want to return a resource encoded as my-private-encoding it's silly to think that any client would be able to accept that. In practice, it's far safer to assume identity encoding only so that clients aren't confronted with unexpected encodings.

craigcondit avatar Mar 21 '24 21:03 craigcondit