Re: [pbs-devel] [PATCH v2 proxmox-backup 3/3] client: reader: signal server before client disconnect

From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Christian Ebner <c.ebner@proxmox.com>,
	Proxmox Backup Server development discussion
	<pbs-devel@lists.proxmox.com>
Subject: Re: [pbs-devel] [PATCH v2 proxmox-backup 3/3] client: reader: signal server before client disconnect
Date: Thu, 05 Dec 2024 12:29:23 +0100	[thread overview]
Message-ID: <1733397994.jmfpi00sd9.astroid@yuna.none> (raw)
In-Reply-To: <b7b3e554-dd98-4568-8991-4433190165c2@proxmox.com>

On December 5, 2024 10:56 am, Christian Ebner wrote:
> On 12/5/24 10:40, Fabian Grünbichler wrote:
>>>> we could even provide some sort of message via the finish API call that
>>>> the server could log if desired, differentiating between:
>>>>
>>>> - regular finish (no error/warning)
>>>> - finish called with a warning message (warning)
>>>> - finish not called, reader went away (error)
>>>>
>>>> ?
>>>
>>> Not sure about this: What warnings do we even want to tell the server
>>> about? I think the reader instance will either work without issues or
>>> fail with a hard error? Or do you have some specific use-case in mind here?
>> 
>> well it would be a way to record on the server that that reader "failed"
>> from a client's point of view, which would then be reflected in the task
>> lists/warning counts/..
>> 
>> e.g., if you do a restore, but abort it in the middle for some reason,
>> you could tell the server so that an admin looking at the server side
>> tasks can tell that this was an "incomplete" read session (this would
>> just be warning case, not an error).
> 
> Hmm, indeed. An intentional abort might be better handled as warning.
> Ideally the same would be possible for the backup writer path as well, 
> so signaling this via additional api endpoint `abort` might even be 
> better? Since the `finish` is already intended to close and check 
> consistency of the backup snapshot for the backup writer case. Just 
> adding a flag to differentiate this seems less declarative.
> 
> Any thoughts on that?

would have to look at the/a concrete implementation, but I think that
should also work for the writer case, since whenever we'd want to call
abort(), we'd not call finish(), but just close the connection, which
already triggers an error on the server side anyway? so even when a
(new) writer calls abort on an old server, just the error message would
change (from whatever it is right now to invalid request or something
like that), but the server-side task would still end up in an error
state anyway.. and for new client + new server, we'd get a nicer state
and message.

_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel