Bug 1670718 - md-cache should be loaded at a position in graph where it sees stats in write cbk [NEEDINFO]
Summary: md-cache should be loaded at a position in graph where it sees stats in write...
Alias: None
Product: GlusterFS
Classification: Community
Component: glusterd
Version: mainline
Hardware: Unspecified
OS: Unspecified
Target Milestone: ---
Assignee: bugs@gluster.org
QA Contact:
Depends On:
Blocks: 1670719 glusterfs-6.0 1732875
TreeView+ depends on / blocked
Reported: 2019-01-30 09:02 UTC by Raghavendra G
Modified: 2020-01-13 10:19 UTC (History)
10 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 1670719 (view as bug list)
Last Closed: 2020-01-13 10:19:38 UTC
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
jahernan: needinfo? (moagrawa)
jahernan: needinfo? (pkarampu)

Attachments (Terms of Use)

System ID Private Priority Status Summary Last Updated
Gluster.org Gerrit 22124 0 None Abandoned performance/md-cache: load as a child of write-behind 2020-03-30 19:30:30 UTC

Description Raghavendra G 2019-01-30 09:02:05 UTC
Description of problem:
The current xlator graph has write-behind as a child of md-cache. When writes are cached, write-behind returns NULL values for stats. So, a write heavy workload essentially removes stats from cache always rendering md-cache useless.

If we load md-cache as a child of write-behind, write cbk will have stats from bricks and hence cache will be updated with latest stat in write workloads.

Version-Release number of selected component (if applicable):

How reproducible:

Steps to Reproduce:

Actual results:

Expected results:

Additional info:

Comment 1 Worker Ant 2019-01-30 09:05:17 UTC
REVIEW: https://review.gluster.org/22124 (performance/md-cache: load as a child of write-behind) posted (#1) for review on master by Raghavendra G

Comment 2 Atin Mukherjee 2019-03-12 05:07:55 UTC
Is this a blocker to release-6? Can we please re-evaluate?

Comment 3 Yaniv Kaul 2019-12-31 07:26:59 UTC
Do we want this in or shall we close it?

Comment 4 Sanju 2019-12-31 07:50:43 UTC
(In reply to Yaniv Kaul from comment #3)
> Do we want this in or shall we close it?

I'm not sure about the importance of this issue. I think we can close it.

Comment 5 Yaniv Kaul 2019-12-31 10:03:58 UTC
(In reply to Sanju from comment #4)
> (In reply to Yaniv Kaul from comment #3)
> > Do we want this in or shall we close it?
> I'm not sure about the importance of this issue. I think we can close it.

From the patch description:
"This benefits write workload as md-cache can absorb fstats calls from kernel." 

I'd like to understand better if there is a benefit here. I'm re-running the regressions tests, to see at least if it's stable first.

Comment 7 Xavi Hernandez 2020-01-09 15:03:55 UTC
The main issue here is that write-behind is returning NULL for post iatt after a cached write. When md-cache sees a NULL iatt, it invalidates its cache. This happens because write-behind is not caching metadata, so it can't provide a meaningful iatt when the write has not been processed by bricks.

On the other side, placing md-cache after write-behind requires that write-behind serializes all operations that return an iatt (virtually all) so that when they reach md-cache (at least those that the user has sent sequentially and write-behind has answered directly), the previous operations have been completely executed and updated the cached metadata. I'm not 100% sure if this is what write-behind is doing in all cases, but doing so is inefficient. For example, if user sends a write request and then an fstat request, write-behind will absorb the write, but when it sees the fstat request, the cached write needs to be flushed and the fstat delayed until the write finishes so that the fstat gets the correct answer.

I think the good approach here would be to unify caching layers and provide an authoritative cache with consistency guarantees. There's a github issue [1] for this, but probably it's a very ambitious approach for a first implementation. Maybe we could get most of the benefits by using a lock based cache that integrates metadata and data. With this approach, an fstat after a write could be served even without flushing the cached write, so both requests could be served directly from client cache with no network/brick activity.

If that's the way to go, I would close this bug and create a new one (or a feature request in github) to implement that caching.

What do you think ?

[1] https://github.com/gluster/glusterfs/issues/218

Comment 8 Yaniv Kaul 2020-01-09 15:24:18 UTC
I'd close and leave the feature request in githbub. This one certainly looks like a sizable project - more than we should commit to right now.

Comment 9 Amar Tumballi 2020-01-13 06:54:06 UTC
+1 on moving the discussion to github with enhancement tag. Lets get the discussion going there, and see when and 'who' can pick this up.

Comment 10 Xavi Hernandez 2020-01-13 10:19:38 UTC
Based on latest comments I'm closing this bug and I've updated the github issue to work on these lines.

[1] https://github.com/gluster/glusterfs/issues/218

Note You need to log in before you can comment on or make changes to this bug.