Skip to content

CP-309718: calculate a moving average of leaf size in GC #782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

CP-309718: calculate a moving average of leaf size in GC #782

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 30 additions & 11 deletions libs/sm/cleanup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2029,7 +2029,7 @@ def _coalesce(self, vdi):
class CoalesceTracker:
GRACE_ITERATIONS = 2
MAX_ITERATIONS_NO_PROGRESS = 3
MAX_ITERATIONS = 10
MAX_ITERATIONS = 20
MAX_INCREASE_FROM_MINIMUM = 1.2
HISTORY_STRING = "Iteration: {its} -- Initial size {initSize}" \
" --> Final size {finSize}"
Expand All @@ -2038,18 +2038,37 @@ def __init__(self, sr):
self.itsNoProgress = 0
self.its = 0
self.minSize = float("inf")
self.history = []
self._history = []
self.reason = ""
self.startSize = None
self.finishSize = None
self.sr = sr
self.grace_remaining = self.GRACE_ITERATIONS

@property
def history(self):
return [x['msg'] for x in self._history]

def moving_average(self):
"""
Calculate a three point moving average
"""
assert len(self._history) >= 3

mv_average = sum([x['finalsize'] for x in self._history]) / len(self._history)
util.SMlog(f'Calculated moving average as {mv_average}')
return mv_average

def abortCoalesce(self, prevSize, curSize):
self.its += 1
self.history.append(self.HISTORY_STRING.format(its=self.its,
initSize=prevSize,
finSize=curSize))
self._history.append(
{
'finalsize': curSize,
'msg': self.HISTORY_STRING.format(its=self.its,
initSize=prevSize,
finSize=curSize)
}
)

self.finishSize = curSize

Expand All @@ -2062,18 +2081,18 @@ def abortCoalesce(self, prevSize, curSize):
if prevSize < self.minSize:
self.minSize = prevSize

if self.its == 1:
# Skip evaluating conditions on first iteration
if self.its < 4:
# Perform at least three iterations
return False

if prevSize < curSize:
if prevSize >= curSize or curSize < self.moving_average():
# We made progress
return False
else:
self.itsNoProgress += 1
Util.log("No progress, attempt:"
" {attempt}".format(attempt=self.itsNoProgress))
util.fistpoint.activate("cleanup_tracker_no_progress", self.sr.uuid)
else:
# We made progress
return False

if self.its > self.MAX_ITERATIONS:
max = self.MAX_ITERATIONS
Expand Down
78 changes: 50 additions & 28 deletions tests/test_cleanup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,7 @@ def test_coalesceLeaf_size_bigger(self, mock_log,
vdi_uuid = uuid4()
vdi = cleanup.VDI(sr, str(vdi_uuid), False)

mock_vhdSize.side_effect = iter([1024, 4096, 4096, 8000, 8000, 16000])
mock_vhdSize.side_effect = iter([1024, 4096, 4096, 8000, 8500, 9000, 9500, 10000, 10500, 16000])

sr._snapshotCoalesce = mock.MagicMock(autospec=True)
sr._snapshotCoalesce.return_value = True
Expand Down Expand Up @@ -1393,16 +1393,16 @@ def autopsyTracker(self, tracker, finalRes, expectedHistory,
self.trackerReportOk(tracker, expectedHistory,
expectedReason, start, finish, minimum)

def exerciseTracker(self, tracker, sizes1, sizes2, its, expectedHistory,
def exerciseTracker(self, tracker, start_sizes, end_sizes, its, expectedHistory,
expectedReason, start, finish, minimum):
for x in range(its):
size1 = sizes1.pop(0)
size2 = sizes2.pop(0)
size1 = start_sizes.pop(0)
size2 = end_sizes.pop(0)
res = tracker.abortCoalesce(size1, size2)
self.assertFalse(res)

size1 = sizes1.pop(0)
size2 = sizes2.pop(0)
size1 = start_sizes.pop(0)
size2 = end_sizes.pop(0)
res = tracker.abortCoalesce(size1, size2)
self.autopsyTracker(tracker, res, expectedHistory, expectedReason,
start, finish, minimum)
Expand All @@ -1425,6 +1425,8 @@ def test_leafCoalesceTracker(self):
"Iteration: 1 -- Initial size 100 --> Final size 100",
"Iteration: 2 -- Initial size 100 --> Final size 121",
"Iteration: 3 -- Initial size 100 --> Final size 141",
"Iteration: 4 -- Initial size 100 --> Final size 150",
"Iteration: 5 -- Initial size 100 --> Final size 160",
]
expectedReason = "Unexpected bump in size," \
" compared to minimum achieved"
Expand All @@ -1433,35 +1435,53 @@ def test_leafCoalesceTracker(self):
res = tracker.abortCoalesce(100, 121)
self.assertFalse(res)
res = tracker.abortCoalesce(100, 141)
self.assertFalse(res)
res = tracker.abortCoalesce(100, 150)
self.assertFalse(res)
res = tracker.abortCoalesce(100, 160)
self.autopsyTracker(tracker, res, expectedHistory,
expectedReason, 100, 141, 100)
expectedReason, 100, 160, 100)

def test_leafCoaleesceTracker_too_many_iterations(self):
"""
Make the GC fail after max iterations
"""
# Note this test is rather brittle if the criteria change
sr = create_cleanup_sr(self.xapi_mock)

# Test initialization
tracker = cleanup.SR.CoalesceTracker(sr)

# 10 iterations no progress 11th fails.
expectedHistory = [
"Iteration: 1 -- Initial size 20 --> Final size 19",
"Iteration: 2 -- Initial size 19 --> Final size 18",
"Iteration: 3 -- Initial size 18 --> Final size 17",
"Iteration: 4 -- Initial size 17 --> Final size 16",
"Iteration: 5 -- Initial size 16 --> Final size 15",
"Iteration: 6 -- Initial size 15 --> Final size 14",
"Iteration: 7 -- Initial size 14 --> Final size 13",
"Iteration: 8 -- Initial size 13 --> Final size 12",
"Iteration: 9 -- Initial size 12 --> Final size 11",
"Iteration: 10 -- Initial size 11 --> Final size 10",
"Iteration: 11 -- Initial size 10 --> Final size 12"
"Iteration: 1 -- Initial size 20 --> Final size 20",
"Iteration: 2 -- Initial size 20 --> Final size 20",
"Iteration: 3 -- Initial size 20 --> Final size 20",
"Iteration: 4 -- Initial size 20 --> Final size 20",
"Iteration: 5 -- Initial size 20 --> Final size 20",
"Iteration: 6 -- Initial size 20 --> Final size 20",
"Iteration: 7 -- Initial size 20 --> Final size 20",
"Iteration: 8 -- Initial size 20 --> Final size 20",
"Iteration: 9 -- Initial size 20 --> Final size 20",
"Iteration: 10 -- Initial size 20 --> Final size 20",
"Iteration: 11 -- Initial size 20 --> Final size 20",
"Iteration: 12 -- Initial size 20 --> Final size 20",
"Iteration: 13 -- Initial size 20 --> Final size 20",
"Iteration: 14 -- Initial size 20 --> Final size 20",
"Iteration: 15 -- Initial size 20 --> Final size 20",
"Iteration: 16 -- Initial size 20 --> Final size 20",
"Iteration: 17 -- Initial size 20 --> Final size 20",
"Iteration: 18 -- Initial size 20 --> Final size 21",
"Iteration: 19 -- Initial size 21 --> Final size 22",
"Iteration: 20 -- Initial size 22 --> Final size 23",
"Iteration: 21 -- Initial size 23 --> Final size 24",
]
expectedReason = "Max iterations (10) exceeded"
expectedReason = "Max iterations (20) exceeded"
self.exerciseTracker(tracker,
[20,19,18,17,16,15,14,13,12,11,10],
[19,18,17,16,15,14,13,12,11,10,12],
10, expectedHistory,
expectedReason, 20, 12, 10)
[20] * 18 + [21, 22, 23],
[20] * 17 + [21, 22, 23, 24],
20, expectedHistory,
expectedReason, 20, 24, 20)

def test_leafCoalesceTracker_getting_bigger(self):
sr = create_cleanup_sr(self.xapi_mock)
Expand All @@ -1475,14 +1495,16 @@ def test_leafCoalesceTracker_getting_bigger(self):
"Iteration: 2 -- Initial size 101 --> Final size 102",
"Iteration: 3 -- Initial size 102 --> Final size 103",
"Iteration: 4 -- Initial size 103 --> Final size 104",
"Iteration: 5 -- Initial size 104 --> Final size 105"
"Iteration: 5 -- Initial size 104 --> Final size 105",
"Iteration: 6 -- Initial size 105 --> Final size 106",
"Iteration: 7 -- Initial size 106 --> Final size 107",
]
expectedReason = "No progress made for 3 iterations"
self.exerciseTracker(tracker,
[100,101,102,103,104],
[101,102,103,104,105],
4, expectedHistory,
expectedReason, 100, 105, 100)
[100,101,102,103,104,105,106],
[101,102,103,104,105,106,107],
6, expectedHistory,
expectedReason, 100, 107, 100)

def runAbortable(self, func, ret, ns, abortTest, pollInterval, timeOut):
return func()
Expand Down
Loading