Bug 247557

Summary: ZFS History Unbounded Memory Usage
Product: Base System Reporter: Sam Vaughan <samjvaughan>
Component: kernAssignee: Mark Johnston <markj>
Status: Closed FIXED    
Severity: Affects Many People CC: markj
Priority: ---    
Version: 12.1-RELEASE   
Hardware: amd64   
OS: Any   

Description Sam Vaughan 2020-06-26 07:41:24 UTC
I have a production server running FreeBSD 11.3 amd64 that has been running out of swap space occasionally at 3:01am.  I went through all of the periodic daily scripts and discovered that it occurs when the /etc/periodic/daily/800.scrub-zfs script runs.

Looking a bit closer, it appears that the `zfs history` command is using 3.7G of memory producing 353k lines of output.  (this server has had many years of regular snapshots)

I have reproduced the issue on another server running FreeBSD 12.1 amd64.  It used about 1.2G of memory producing 104k lines of output.

After a short search I uncovered an OpenZFS commit that addresses this issue by processing the history in chunks:

https://github.com/openzfs/zfs/pull/9516/commits/d6156a01ba93e0eee6fec64c0adc8e4740673718

I then browsed the FreeBSD sources and saw that this commit has not made it to the FreeBSD codebase yet.

I don't know what the situation is with all the different ZFSes and merging of changes but I'm wondering if one of the developers would be kind enough to try merging this fix please?

It would be awesome if this issue could be addressed because it has the potential to make production systems unstable over time as their ZFS history grows.
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2020-06-26 07:45:15 UTC
This may need an MFV.
Comment 2 Mark Johnston freebsd_committer freebsd_triage 2020-07-20 16:53:48 UTC
I posted a patch here, if you are interested in testing: https://reviews.freebsd.org/D25745
Comment 3 Sam Vaughan 2020-07-20 23:01:06 UTC
Thanks so much, Mark.  I would be more than happy to test on my servers but I need to learn how to build and install.  Is it possible to just build the zpool binary and libzfs library, then swap them in?

I had a look through the official handbook and developer documentation but haven't yet found docs for core system user space build/install/test.  I'll keep looking.
Comment 4 Mark Johnston freebsd_committer freebsd_triage 2020-07-21 15:50:56 UTC
(In reply to Sam Vaughan from comment #3)
Yes, you can build libzfs and zpool from /usr/src/cddl/lib/libzfs and /usr/src/cddl/sbin/zpool, respectively.  If the source tree matches your FreeBSD installation it should be possible to simply "make && make install" from those directories.

Note though that the patch modifies the ABI of libzfs, so any other applications that link against it and use it to fetch zpool history would need to be modified.  I think the risk of actually breaking something is pretty low (there are no other users of zpool_get_history() in the base system, for instance), but it's possible, especially if you have any custom utilities that link against libzfs.
Comment 5 Sam Vaughan 2020-07-23 06:37:00 UTC
I can confirm that your patch works, Mark, thank you very much.

It has made a massive difference to the performance of the `zpool history` command.  The resident set size reported by `/usr/bin/time -l zpool history` dropped from 884M to 9M and execution time halved from 12.9s to 6.6s.

Here's what I did to test.  Please let me know if you need any more info or if my methodology wasn't right:

# uname -imrs
FreeBSD 12.1-RELEASE-p7 amd64 GENERIC

# /usr/bin/time -l zpool history > zpool_12.1p7_history.txt
       12.90 real         4.29 user         0.99 sys
    884144  maximum resident set size
        55  average shared memory size
        15  average unshared data size
       127  average unshared stack size
    222765  page reclaims
         0  page faults
         0  swaps
       803  block input operations
        55  block output operations
         0  messages sent
         0  messages received
         0  signals received
       254  voluntary context switches
      2280  involuntary context switches

# sudo svnlite co https://svn.freebsd.org/base/releng/12.1 /usr/src
# cd /usr/src
# patch -p0 < D25745.diff
# cd /usr/src/cddl/lib/libzfs
# make && make install
# cd /usr/src/cddl/sbin/zpool
# make && make install
# cd

# /usr/bin/time -l zpool history > zpool_12.1p7_patched_history.txt
        6.59 real         3.66 user         0.34 sys
      9384  maximum resident set size
        55  average shared memory size
        15  average unshared data size
       127  average unshared stack size
      1672  page reclaims
        10  page faults
         0  swaps
         0  block input operations
        55  block output operations
         0  messages sent
         0  messages received
         0  signals received
        29  voluntary context switches
      3656  involuntary context switches

# wc -l zpool*
  105842 zpool_12.1p7_history.txt
  105842 zpool_12.1p7_patched_history.txt
  211684 total
# md5 zpool*
MD5 (zpool_12.1p7_history.txt) = 95837a4490779d5ec10cbb8d1f119dc9
MD5 (zpool_12.1p7_patched_history.txt) = 95837a4490779d5ec10cbb8d1f119dc9
Comment 6 Mark Johnston freebsd_committer freebsd_triage 2020-07-23 14:15:23 UTC
(In reply to Sam Vaughan from comment #5)
Perfect, thanks for testing.  I tried it on my pool but the history is not nearly long enough to trigger any "interesting" behaviour.
Comment 7 commit-hook freebsd_committer freebsd_triage 2020-07-23 14:22:18 UTC
A commit references this bug:

Author: markj
Date: Thu Jul 23 14:21:46 UTC 2020
New revision: 363447
URL: https://svnweb.freebsd.org/changeset/base/363447

Log:
  MFOpenZFS: Fix zpool history unbounded memory usage

  In original implementation, zpool history will read the whole history
  before printing anything, causing memory usage goes unbounded. We fix
  this by breaking it into read-print iterations.

  Reviewed-by: Tom Caputi <tcaputi@datto.com>
  Reviewed-by: Matt Ahrens <matt@delphix.com>
  Reviewed-by: Igor Kozhukhov <igor@dilos.org>
  Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
  Signed-off-by: Chunwei Chen <david.chen@nutanix.com>
  Closes #9516

  Note, this change changes the libzfs.so ABI by modifying the prototype
  of zpool_get_history().  Since libzfs is effectively private to the base
  system it is anticipated that this will not be a problem.

  PR:		247557
  Obtained from:	OpenZFS
  Reported and tested by:	Sam Vaughan <samjvaughan@gmail.com>
  Discussed with:	freqlabs
  MFC after:	2 weeks
  Differential Revision:	https://reviews.freebsd.org/D25745
  openzfs/zfs@7125a109dcc55628336ff3f58e59e503f4d7694d

Changes:
  head/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c
  head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h
  head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c
Comment 8 commit-hook freebsd_committer freebsd_triage 2020-08-06 14:08:14 UTC
A commit references this bug:

Author: markj
Date: Thu Aug  6 14:07:50 UTC 2020
New revision: 363952
URL: https://svnweb.freebsd.org/changeset/base/363952

Log:
  MFC r363447:
  MFOpenZFS: Fix zpool history unbounded memory usage

  PR:	247557

Changes:
_U  stable/12/
  stable/12/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c
  stable/12/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h
  stable/12/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c
Comment 9 commit-hook freebsd_committer freebsd_triage 2020-08-06 14:16:16 UTC
A commit references this bug:

Author: markj
Date: Thu Aug  6 14:16:08 UTC 2020
New revision: 363954
URL: https://svnweb.freebsd.org/changeset/base/363954

Log:
  MFC r363447:
  MFOpenZFS: Fix zpool history unbounded memory usage

  PR:	247557

Changes:
_U  stable/11/
  stable/11/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c
  stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h
  stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c
Comment 10 Mark Johnston freebsd_committer freebsd_triage 2020-08-06 14:16:37 UTC
Thanks for the report.