From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <t.lamprecht@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits))
(No client certificate requested)
by lists.proxmox.com (Postfix) with ESMTPS id 581B981885
for <pve-devel@lists.proxmox.com>; Wed, 24 Nov 2021 18:46:37 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 45AFD231F1
for <pve-devel@lists.proxmox.com>; Wed, 24 Nov 2021 18:46:07 +0100 (CET)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
[94.136.29.106])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits))
(No client certificate requested)
by firstgate.proxmox.com (Proxmox) with ESMTPS id 57BDE231E3
for <pve-devel@lists.proxmox.com>; Wed, 24 Nov 2021 18:46:06 +0100 (CET)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 25BE4447E2
for <pve-devel@lists.proxmox.com>; Wed, 24 Nov 2021 18:46:06 +0100 (CET)
Message-ID: <1e36e64e-e8bc-940e-df9e-1079a353463c@proxmox.com>
Date: Wed, 24 Nov 2021 18:46:04 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:95.0) Gecko/20100101
Thunderbird/95.0
Content-Language: en-US
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Stoiko Ivanov <s.ivanov@proxmox.com>, Dominik Csapak <d.csapak@proxmox.com>
References: <20211124144748.68687-1-d.csapak@proxmox.com>
<20211124144748.68687-4-d.csapak@proxmox.com>
<20211124183237.6e6fc40b@rosa.proxmox.com>
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
In-Reply-To: <20211124183237.6e6fc40b@rosa.proxmox.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results: 0
AWL 1.863 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
NICE_REPLY_A -3.515 Looks like a legit reply (A)
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH manager 1/1] api: journal: stream the
journal data to the client
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Wed, 24 Nov 2021 17:46:37 -0000
On 24.11.21 18:32, Stoiko Ivanov wrote:
> Huge thanks for addressing this so quickly and elegantly!
>
> One question/nit:
>
> On Wed, 24 Nov 2021 15:47:48 +0100
> Dominik Csapak <d.csapak@proxmox.com> wrote:
>
>> instead of accumulating the whole output of 'mini-journalreader' in
>> the api call (this can be quite big), use the download mechanic of the
>> http-server to stream the output to the client.
>>
>> we lose some error handling possibilities, but we do not have
>> to allocate anything here, and since perl does not free memory after
>> allocating[0] this is our desired behaviour.
>>
>> to keep api compatiblitiy, we need to give the journalreader the '-j'
>> flag to let it output json.
>>
>> also tell the http server that the encoding is gzip and pipe
>> the output through it.
>>
>> 0: https://perldoc.perl.org/perlfaq3#How-can-I-free-an-array-or-hash-so-my-program-shrinks?
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>> PVE/API2/Nodes.pm | 22 ++++++++++++++--------
>> 1 file changed, 14 insertions(+), 8 deletions(-)
>>
>> diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
>> index 565cbccc..d57a1937 100644
>> --- a/PVE/API2/Nodes.pm
>> +++ b/PVE/API2/Nodes.pm
>> @@ -819,19 +819,25 @@ __PACKAGE__->register_method({
>> my $rpcenv = PVE::RPCEnvironment::get();
>> my $user = $rpcenv->get_user();
>>
>> - my $cmd = ["/usr/bin/mini-journalreader"];
>> + my $cmd = ["/usr/bin/mini-journalreader", "-j"];
>> push @$cmd, '-n', $param->{lastentries} if $param->{lastentries};
>> push @$cmd, '-b', $param->{since} if $param->{since};
>> push @$cmd, '-e', $param->{until} if $param->{until};
>> - push @$cmd, '-f', $param->{startcursor} if $param->{startcursor};
>> - push @$cmd, '-t', $param->{endcursor} if $param->{endcursor};
>> + push @$cmd, '-f', PVE::Tools::shellquote($param->{startcursor}) if $param->{startcursor};
>> + push @$cmd, '-t', PVE::Tools::shellquote($param->{endcursor}) if $param->{endcursor};
>> + push @$cmd, ' | gzip ';
> Not sure which would be more efficient - but the http-server does support
> gzipping the result from the API
It has no api result in that case, it just returns the open file handle and anyevent
can then stream that directly, see the response_stream method in PVE::APIServer::AnyEvent.
> might it make sense to let the worker do the gzip-encoding vs. forking
> gzip here?
contrary to our rust infra from PBS we do not have a streaming encoder directly in
the server that we can just layer in between, pve-http-server can currently only compress
if it has the whole response, and that's, which is what we want to avoid here.
The response method explicitly guards for the stream-an-fh case, AnyEvent.pm line 325:
if ($content && !$stream_fh) {
# ...
my $comp = Compress::Zlib::memGzip($content);
What we can do though is compress directly in the mini-journal reader, but that'll be
way easier once we rewrite in in rust (heh), as then we can reuse our existing
infrastructure to just layer that in-between.