braid-org / braid-spec

Working area for Braid extensions to HTTP
https://braid.org
233 stars 16 forks source link

Simplify `Subscribe: keep-alive` #80

Closed canadaduane closed 1 year ago

canadaduane commented 3 years ago

In section 3.1 it says:

   If a client requests "Subscribe: keep-alive", then the subscription will be
   remembered even after the GET connection closes.  A subscription can
   be resumed by the client issuing another GET with a Subscribe header.

Two concerns:

  1. This seems like it could open a DDoS attack surface--if many clients start subscribing and dropping connections, the server seems obligated to keep (possibly millions of) maybe-the-client-will-reconnect data around.
  2. Shouldn't the client always have enough information to tell the server, on subscribe, where to start? In this case, why make the server remember state at all?
canadaduane commented 3 years ago

I see from section 3.3 that my concerns are mostly addressed:

   Even if a connection closes, a subscription might still be active.
   If a server's response headers for a connection contained:

           Subscribe: keep-alive
      or   Subscribe: keep-alive=<seconds>

   Then the server SHOULD keep the subscription open even after the
   connection closes.  This means that the server promises to keep
   enough history to merge with the client when the client comes back
   online.

So it sounds like the idea is that a server's "maximum memory footprint" over the set of all clients that subscribe to it is simply whatever data the server is storing for state, and related history. I.e. there is no information specific to each client. Is that right?

toomim commented 3 years ago

Thanks Duane! That text in section 3.1 looks misleading to me, and I think we should clarify it. It implies that the client controls how long the subscription will stay open—but our intent when writing this was for the server to make the final call.

As to whether the server needs to store information specific to each client — that's up to the server! The spec doesn't force servers to obey subscription requests, nor does it say how the information should be stored. I can imagine server implementations that don't keep information specific to each client, and implementations that do.

mitar commented 3 years ago

So language should be:

   If a client requests "Subscribe: keep-alive", then the subscription CAN be
   remembered even after the GET connection closes.  A subscription CAN
   be resumed by the client issuing another GET with a Subscribe header.

So see CAN there? Or we could use some other similar word.

josephg commented 3 years ago

MAY might read better

toomim commented 3 years ago

Consensus:

In the future, it's possible that we could invent other parameters for Subscribe, such as:

Subscribe: duration=connection, update=snapshot,patch

The update=snapshot,patch attribute is relevant to @brynbellomy's redwood protocol work as well.

We still need to write a PR to change this.

toomim commented 1 year ago

I wrote a PR for this at https://github.com/braid-org/braid-spec/pull/122.

Note that I broke from consensus in a sense by writing all the examples with blank Subscribe: headers. The description suggests Subscribe: true, but I fear implementors blindly requiring true on a server without reading that true is optional, and then the servers will break if clients start replacing true with more nuanced parameters, instead of gracefully ignoring the parameters that they don't understand.

toomim commented 1 year ago

Closing this issue. All discussion of subscription parameters are now being consolidated to https://github.com/braid-org/braid-spec/issues/123.