Skip to content

Fix dmu_tx_dirty_throttle after initial arc_c reduction. #11178

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

Merged
merged 1 commit into from
Nov 10, 2020

Conversation

amotin
Copy link
Member

@amotin amotin commented Nov 8, 2020

After initial arc_c was reduced to arc_c_min it became possible that on datasets with primarycache=metadata or none dirty data make up most of ARC capacity and easily more than configured 50% of initial arc_c, that causes forced txg commits by arc_tempreserve_space() and periodic very long write delays.

This patch makes arc_tempreserve_space() to use arc_c only after ARC warmed up once and arc_c really means something, but use arc_c_max before that.

How Has This Been Tested?

On freshly booted FreeBSD head created dataset with primarycache=metadata and started writing it sequentially while pressing Ctrl+T to see its call stack and measuring the write call latency. With the patch I see proper write throttling and pretty stable latency about 1ms instead of regular delay jumps up to 5-6 seconds otherwise, waiting for TXG commits.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist:

After initial arc_c was reduced to arc_c_min it became possible that
on datasets with primarycache=metadata or none dirty data make up most
of ARC capacity and easily more than configured 50% of initial arc_c,
that causes forced txg commits by arc_tempreserve_space() and periodic
very long write delays.

This patch makes arc_tempreserve_space() to use arc_c only after ARC
warmed up once and arc_c really means something, but use arc_c_max
before that.

Signed-off-by: Alexander Motin <[email protected]>
Sponsored-By: iXsystems, Inc.
@amotin amotin added the Status: Code Review Needed Ready for review and testing label Nov 8, 2020
@amotin amotin self-assigned this Nov 8, 2020
@amotin amotin requested review from a user and behlendorf November 8, 2020 02:06
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

I wonder where else this logic might be applicable.
l2arc_hdr_limit_reached has arc_warm ? arc_c : arc_c_max as well.

static boolean_t
l2arc_hdr_limit_reached(void)
{
        int64_t s = aggsum_upper_bound(&astat_l2_hdr_size);

        return (arc_reclaim_needed() || (s > arc_meta_limit * 3 / 4) ||
            (s > (arc_warm ? arc_c : arc_c_max) * l2arc_meta_percent / 100));
}

@amotin
Copy link
Member Author

amotin commented Nov 9, 2020

@freqlabs I've looked through all other arc_c use cases and found no candidates other than those two.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Nov 9, 2020
@behlendorf behlendorf merged commit daabdda into openzfs:master Nov 10, 2020
behlendorf pushed a commit that referenced this pull request Nov 11, 2020
After initial arc_c was reduced to arc_c_min it became possible that
on datasets with primarycache=metadata or none dirty data make up most
of ARC capacity and easily more than configured 50% of initial arc_c,
that causes forced txg commits by arc_tempreserve_space() and periodic
very long write delays.

This patch makes arc_tempreserve_space() to use arc_c only after ARC
warmed up once and arc_c really means something, but use arc_c_max
before that.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Matt Macy <[email protected]>
Signed-off-by: Alexander Motin <[email protected]>
Sponsored-By: iXsystems, Inc.
Closes #11178
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
After initial arc_c was reduced to arc_c_min it became possible that
on datasets with primarycache=metadata or none dirty data make up most
of ARC capacity and easily more than configured 50% of initial arc_c,
that causes forced txg commits by arc_tempreserve_space() and periodic
very long write delays.

This patch makes arc_tempreserve_space() to use arc_c only after ARC
warmed up once and arc_c really means something, but use arc_c_max
before that.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Matt Macy <[email protected]>
Signed-off-by: Alexander Motin <[email protected]>
Sponsored-By: iXsystems, Inc.
Closes openzfs#11178
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
After initial arc_c was reduced to arc_c_min it became possible that
on datasets with primarycache=metadata or none dirty data make up most
of ARC capacity and easily more than configured 50% of initial arc_c,
that causes forced txg commits by arc_tempreserve_space() and periodic
very long write delays.

This patch makes arc_tempreserve_space() to use arc_c only after ARC
warmed up once and arc_c really means something, but use arc_c_max
before that.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Matt Macy <[email protected]>
Signed-off-by: Alexander Motin <[email protected]>
Sponsored-By: iXsystems, Inc.
Closes openzfs#11178
@amotin amotin deleted the dmu_tx_dirty_throttle branch August 24, 2021 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants