Support no_erase on message delete

[scottf]

Overview

For the message delete request STREAM.MSG.DELETE.%s the payload json has been expanded to contain a no_erase field.

• The false/default setting overwrites the message deleted by the api with garbage
• Setting no_erase to true only marks it as erased only

There is a significant impact on performance

type JSApiMsgDeleteRequest struct {
    Seq     uint64 `json:"seq"`
    NoErase bool   `json:"no_erase,omitempty"`
}

The behavior is documented in ADR-X.

Clients and Tools

• [x] Schemas @ripienaar
• [x] Go @piotrpio
• [x] Java @scottf
• [x] JavaScript @aricart (https://github.com/nats-io/nats.deno/pull/117)
• [x] .Net @scottf
• [x] C @kozlovic
• [ ] Python @wallyqs
• [ ] Ruby @wallyqs
• [ ] Rust @Jarema

Other Tasks

• [ ] docs.nats.io updated @jnmoyne
• [ ] Update client features spreadsheet

Client authors please update with your progress. If you open issues in your own repositories as a result of this request, please link them to this one by pasting the issue URL in a comment or main issue description.

[scottf]

@derek said this in slack:

By default the delete should be like limits, secure erasing should be opt in imo. Been there since launch of JetStream I believe for GDPR and "right to be forgotten" issue per Colin..

We could handle this on the client by setting no_erase to true by default. I suppose the server could also default no_erase to true when empty, but for a flag that seems bad form. It appears that our clients except for the c client have not coded for this, I suppose this is a chance to switch the flag in the server to something along the lines of yes_erase which will require the user to forceably set it. Also, what about a stream configuration? Seems like something useful to set at the stream level, which would allow empty no_erase to be interpreted as whatever the stream setting is, but then clients would have to make sure they always send the flag, instead of what is typically done which is not sending false

[ripienaar]

Whats the problem with the default as is? It's doing the most correct thing by default, allowing those who want performance at the expense of safety to opt into that.

We dont have many safe-by-default settings so I think we'd need v.good reasons to change it.

[ripienaar]

Scott asked me to expand on my comment.

Many of our flags are there because we find some problem, so we add a flag where a user can opt-into safety - setting some limit like max expires, max waiting, idle and fc. All of these are critical to set for the proper operation of the system, most shouldnt even be options. They are all opt in though and some so obtuse that I cant imagine any user really understands them or how to pick a legit value for them. The outcome is it's bad for everyone by default and so few users know how to use it correctly that those settings might as well not be there.

Additionally all these flags - that should be defaults - just massively clutter up all user interfaces, client libraries, status views and more.

The outcome is by default its either succeptible to abuse or worse promote user error that can have dire consequences.

This no_erase setting is a shining example of the opposite. It has the default that protects everyone and does the right thing in most of the cases. It's easy to disover in a client because it would be a flag on the DeleteMsg function, or a second appropriately named function, and easily documented in place in api docs, making it discoverable and understandable exactly at the point of use.

Contrast that with effect-at-a-distance of a Stream flag that influences consumer behavior or the behavior of some other API call, its a really bad idea because its hard to discover when adding a consumer what it is or worse while calling an API you dont know what the API will do, it explodes our flags leading to even more option fatigue.

In the specific case of no_erase a developer might write function gdprErase(msgId) thinking great they are doing the right thing, now someone edits the stream configuration and suddently their gdprErase() function becomes lawSuitWaitingToHappen() without any code changes.

Imo, this flag and feature is exactly perfectly designed and provide a opt-out into dangerous territories rather than a opt-into safety. perfect.

[derekcollison]

More people will care deeply about the performance of Delete then a Secure Delete imo. We will entertain more support questions and issues on that versus "What really happens when you do a delete from the client, and why is that different then when the system deletes messages on its own".

It was designed really to be its own function, SecureDelete().

[scottf]

How would people feel about this: In JetStream Management, change DeleteMessage to set no_erase to true and add SecureDeleteMessage which sets no_erase to false or leaves it empty ? +/-

[ripienaar]

would be a API breaking change - otherwise thats fine

[scottf]

Another option would be to leave DeleteMessage as is, and add InsecureDeleteMessage

[derekcollison]

I think change behavior, document well, and add SecureDelete

[aricart]

For the JavaScript clients this has been implemented since 3/21/21 - and by default, it does an erase, which as described above is flips it from secure, to performance. Can change either way, but note that on JavaScript, the erase argument is optional, and by default set to true.

async deleteMessage(
    stream: string,
    seq: number,
    erase = true,
  ): Promise<boolean>