[CASSANDRA-20457] Limit number of OOM heap dumps to avoid full disk issue

[gowa]

New env vars are introduced:
CASSANDRA_HEAPDUMP_CLEAN:
1 - clean (default)
0 - do not clean
CASSANDRA_HEAPDUMP_KEEP_LAST_N_FILES
-1 - no heap dump files will be removed
0 - all heap dump files will be removed
N > 0 - up to N most recent files will be kept
default: 2
CASSANDRA_HEAPDUMP_KEEP_FIRST_N_FILES
-1 - no heap dump files will be removed
0 - all heap dump files will be removed
N > 0 - up to N least recent files will be kept
default: 1

test script

#!/bin/sh

export CASSANDRA_HOME="."
export CASSANDRA_HEAPDUMP_DIR=test_heapdumps

create_hprof_files()
{
  rm -rf test_heapdumps
  mkdir -p test_heapdumps
  if [ ! -d test_heapdumps ]; then
      echo "was not able to create dir test_heapdumps"
  fi

  for i in $(seq 1 1000); do echo 1 > "test_heapdumps/cassandra-$i-pid$i.hprof"; sleep 0.005; done;
  echo 1 > "test_heapdumps/cassandra-1-pid1.hprofX"
  echo 1 > "test_heapdumps/Xcassandra-1-pid1.hprof"
  mkdir -p "test_heapdumps/cassandra-1001-pid1001.hprof"
  if [ ! -d "test_heapdumps/cassandra-1001-pid1001.hprof" ]; then
      echo "was not able to create dir test_heapdumps/cassandra-1001-pid1001.hprof"
  fi
  if ! ls -pt1 test_heapdumps | wc -l | awk '{if ($0 != 1003) exit 1}'; then
    echo "wrong content created under test_heapdumps"
  fi
}

(
  create_hprof_files
  . ./cassandra-env.sh
  clean_heap_dump_files
  if ! ls -pt1 test_heapdumps | wc -l | awk '{if ($0 != 3+3) { print $0; exit 1; }}'; then
    echo "ERROR: 3+3 files/dirs should have left under test_heapdumps"
  fi
  ls -pt1 test_heapdumps
)

(
  create_hprof_files
  CASSANDRA_HEAPDUMP_CLEAN=0
  . ./cassandra-env.sh
  clean_heap_dump_files
  if ! ls -pt1 test_heapdumps | wc -l | awk '{if ($0 != 1003) { print $0; exit 1; }}'; then
    echo "ERROR: 1003 files/dirs should have left under test_heapdumps"
  fi
  ls -pt1 test_heapdumps
)

(
  create_hprof_files
  CASSANDRA_HEAPDUMP_CLEAN=1
  CASSANDRA_HEAPDUMP_KEEP_LAST_N_FILES=3
  CASSANDRA_HEAPDUMP_KEEP_FIRST_N_FILES=3
  . ./cassandra-env.sh
  clean_heap_dump_files
  if ! ls -pt1 test_heapdumps | wc -l | awk '{if ($0 != 3+6) { print $0; exit 1; }}'; then
    echo "ERROR: 3+6 files/dirs should have left under test_heapdumps"
  fi
  ls -pt1 test_heapdumps
)

(
  if [ "x$clean_heap_dump_files_defined" = "x1" ]; then
    echo "error: calling clean_heap_dump_files, not expected"
    clean_heap_dump_files
  fi
)

rm -rf test_heapdumps

[smiklosovic]

I believe there is a simpler way to do this. You can use tail -n and head -n or similar.

[gowa]

Hi @smiklosovic ! Thanks for your review. Will try to address your comments properly.
I decided to rely on awk because it is used already in this script and because the solution using head/tail is not portable (at least chatgpt reported that head's negative line numbers cannot be used on MacOS, while some switch on uname in the script above suggests it should be portable), so I decided to go with awk using its basics only.

[gowa]

kept awk, removed find, xargs, head -n. using ls -pt1 as suggested.

[smiklosovic]

If we simplify this patch to just one property as suggested above, that is "keep newest n and remove the rest" is not it true that awk will not be necessary anymore and we can do with head -n N, N being number of newest files to keep? head -n is portable.

[smiklosovic]

@gowa Why do we even have this property? "keep newest n files" makes sense to me, why would we want to keep oldest n files? Under what circumstance an operator would want to keep oldest around and remove the newest? Don't you think that keeping around just the latest one (or few latest ones) make more sense? This complicates the logic unnecessarily.

I would also rename the properties to have "oldest / newest" in them. I am not sure if "last" and "first" was understood correctly here. "last" means as newest or oldest? It would be cool to know how properties are functioning without consulting the documentation above.

[gowa]

@smiklosovic, in my experience it was often important to see and analyze the first occurrence of the issue, so you know the time and can think of what has changed in the process and led to OOM, while reoccurrence often happens 'automatically'.
It is a heuristic anyway. I suggested this property just based on my gut feeling. I felt it would be beneficial to have a very first heap dump in series.

It can be removed and awk can be eliminated. I suggested an option. If it does not look right, I will drop it.

Agree about last/first.

[smiklosovic]

@netudima you thought about having two properties to keep both newest / oldest?

[smiklosovic]

@gowa why don't you just remove x from both sides? I have never seen this construct. I would also rename it to should_clean_heap_dump_files instead of _defined suffix.