Skip to content
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.

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

FT-689 : Add option to allow disabling of partial eviction of nodes #297

Merged
merged 1 commit into from Aug 27, 2015

Conversation

george-lorch
Copy link
Contributor

Requested by Vadim T. - "For start I propose we create an option --tokudb-enable-partial-eviction (ON by default)."

Jenkins having issues now so no jenkins matrix test. local ctest passed as expected.

@george-lorch george-lorch self-assigned this Jul 30, 2015
@george-lorch george-lorch added this to the tokudb 7.5.9 milestone Jul 30, 2015
@george-lorch george-lorch force-pushed the FT-689 branch 2 times, most recently from 331dfd7 to 8e9b37d Compare July 30, 2015 22:23
m_enable_partial_eviction = enabled;
}

bool evictor::get_enable_partial_eviction(void) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would consider making this method const.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I already have done this per our Slack conversation. I haven't pushed it yet as I'm still working on introducing a test case that validates disabling of partial evictions. It would also be a good idea to test compress_buffers_before_eviction since there is no test for it that I could find and it is not documented anywhere.

@christianrober
Copy link
Contributor

Looks good to me.

Requested by Vadim T. - "For start I propose we create an option --tokudb-enable-partial-eviction (ON by default)."
Modified ft/tests/cachetable-5097 to take parameters and validate proper eviction behavior with and without partial eviction enabled.
@george-lorch
Copy link
Contributor Author

Alrighty, made const and modified a test case to validate the new functionality. ctest passes locally.

christianrober added a commit that referenced this pull request Aug 27, 2015
FT-689 : Add option to allow disabling of partial eviction of nodes
@christianrober christianrober merged commit 458ab99 into percona:master Aug 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants