Skip to content
This repository has been archived by the owner on Nov 14, 2024. It is now read-only.

[PDS-146088] Force-clear the pool if no evictions are happening #5471

Closed
wants to merge 17 commits into from

Conversation

Jolyon-S
Copy link
Contributor

@Jolyon-S Jolyon-S commented May 27, 2021

Goals (and why):
One symptom of PDS-146088 is that the client pool stops evicting. Force-clearing the pool if the eviction task has not even considered anything for eviction after some period of time is one approach that might work.

Implementation Description (bullets):
Clear the client pool if the last successful eviction run was over 15 minutes ago.

Testing (What was existing testing like? What have you done to improve it?):
No tests, which is perhaps dangerous, however I'm not sure what testing would or should look like here.

Concerns (what feedback would you like?):
As it stands, this PR is really bad - we need to also reset the last evicted time in the evictor, and the decomp is pretty wacky. I just wanted to get the concept down before I forgot.

Where should we start reviewing?:
CCPC.

Priority (whenever / two weeks / yesterday):
ASAP - we should get an RC out soon for internal testing.

@Jolyon-S Jolyon-S changed the title [WIP] [PDS-146088] Force-clear the pool if no evictions are happening [PDS-146088] Force-clear the pool if no evictions are happening Jun 1, 2021
@changelog-app
Copy link

changelog-app bot commented Jun 1, 2021

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Force-evict the Cassandra client pool if no eviction has occurred for a while (by default, 60 * timeBetweenConnectionEvictionRunsSeconds, which makes this duration 20 minutes).

Check the box to generate changelog(s)

  • Generate changelog entry

Copy link
Contributor

@jeremyk-91 jeremyk-91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Broadly makes sense. Two small suggested changes, and another that's more for if we want to productionise this. With those I'm not opposed to an RC, we can cut that tomorrow.

@Jolyon-S Jolyon-S closed this Jun 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants