Received-SPF: pass (google.com: domain of linux-nfs+bounces-3484-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <440dcc5a-fdea-4677-9bad-b782e9801747@grimberg.me>
Date: Thu, 30 May 2024 10:44:01 +0300
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: NFS write congestion size
To: Trond Myklebust <trondmy@hammerspace.com>, "jack@suse.cz" <jack@suse.cz>,
 "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Cc: "anna@kernel.org" <anna@kernel.org>, "nfbrown@suse.com" <nfbrown@suse.com>
References: <20240529161102.5x3hhnbz32lwjcej@quack3>
 <4a4368fbc260b22ff96593cedc53954b2cbe47fd.camel@hammerspace.com>
Content-Language: en-US
From: Sagi Grimberg <sagi@grimberg.me>
In-Reply-To: <4a4368fbc260b22ff96593cedc53954b2cbe47fd.camel@hammerspace.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit


On 29/05/2024 20:05, Trond Myklebust wrote:
> On Wed, 2024-05-29 at 18:11 +0200, Jan Kara wrote:
>> Hello,
>>
>> so I was investigating why random writes to a large file over NFS got
>> noticeably slower. The workload we use to test this is this fio
>> command:
>>
>> fio --direct=0 --ioengine=sync --thread --directory=/mnt --
>> invalidate=1 \
>>      --group_reporting=1 --runtime=300 --fallocate=posix --
>> ramp_time=10 \
>>      --name=RandomWrites-async-257024-4k-4 --new_group --rw=randwrite
>> \
>>      --size=32000m --numjobs=4 --bs=4k --fsync_on_close=1 --
>> end_fsync=1 \
>>      --filename_format='FioWorkloads.$jobnum'
>>
>> Eventually I've tracked down the regression to be caused by
>> 6df25e58532b
>> ("nfs: remove reliance on bdi congestion") which changed the
>> congestion
>> mechanism from a generic bdi congestion handling to NFS private one.
>> Before
>> this commit the fio achieved throughput of 180 MB/s, after this
>> commit only
>> 120 MB/s. Now part of the regression was actually caused by
>> inefficient
>> fsync(2) and the fact that more dirty data was cached at the time of
>> the
>> last fsync after commit 6df25e58532b. After fixing fsync [1], the
>> throughput got to 150 MB/s so better but still not quite the
>> throughput
>> before 6df25e58532b.
>>
>> The reason for remaining regression is that bdi congestion handling
>> was
>> broken and the client had happily ~8GB of outstanding IO against the
>> server
>> despite the congestion limit was 256 MB. The new congestion handling
>> actually works but as a result the server does not have enough dirty
>> data
>> to efficiently operate on and the server disk often gets idle before
>> the
>> client can send more.
>>
>> I wanted to discuss possible solutions here.
>>
>> Generally 256MB is not enough even for consumer grade contemporary
>> disks to
>> max out throughput. There is tunable
>> /proc/sys/fs/nfs/nfs_congestion_kb.
>> If I tweak it to say 1GB, that is enough to give the server enough
>> data to
>> saturate the disk (most of the time) and fio reaches 180MB/s as
>> before
>> commit 6df25e58532b. So one solution to the problem would be to
>> change the
>> default of nfs_congestion_kb to 1GB.
>>
>> Generally the problem with this tuning is that faster disks may need
>> even
>> larger nfs_congestion_kb, the NFS client has no way of knowing what
>> the
>> right value of nfs_congestion_kb is. I personally find the concept of
>> client throttling writes to the server flawed. The *server* should
>> push
>> back (or throttle) if the client is too aggressively pushing out the
>> data
>> and then the client can react to this backpressure. Because only the
>> server
>> knows how much it can handle (also given the load from other
>> clients). And
>> I believe this is actually what is happening in practice (e.g. when I
>> tune
>> nfs_congestion_kb to really high number). So I think even better
>> solution
>> may be to just remove the write congestion handling from the client
>> completely. The history before commit 6df25e58532b, when congestion
>> was
>> effectively ignored, shows that this is unlikely to cause any
>> practical
>> problems. What do people think?
> I think we do still need a mechanism to prevent the client from pushing
> more writebacks into the RPC layer when the server throttling is
> causing RPC transmission queues to build up. Otherwise we end up
> increasing the latency when the application is trying to do more I/O to
> pages that are queued up for writeback in the RPC layer (since the
> latter will be write locked).

Plus the server is likely serving multiple clients, so removing any type
of congestion handling from the client may overwhelm the server.