Skip to content

rgw: add negative cache to the system object#35777

Merged
cbodley merged 1 commit intoceph:masterfrom
ofriedma:rgw-enoent-cache
Jul 16, 2020
Merged

rgw: add negative cache to the system object#35777
cbodley merged 1 commit intoceph:masterfrom
ofriedma:rgw-enoent-cache

Conversation

@ofriedma
Copy link
Contributor

add negative cache to the system object

Signed-off-by: Or Friedmann ofriedma@redhat.com

Fixes: https://tracker.ceph.com/issues/45816

Copy link
Contributor

@mattbenjamin mattbenjamin left a comment

Choose a reason for hiding this comment

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

this looks potentially good--can you do a teuthology run?

int status = 0;
uint32_t flags = 0;
uint64_t epoch = 0;
bool noentry = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't this just duplicate the int status field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

if(perfcounter) perfcounter->inc(l_rgw_cache_miss);
if(src.noentry) {
ldout(cct, 20) << "negative cache entry" << dendl;
return -ENODATA;
Copy link
Contributor

Choose a reason for hiding this comment

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

this shouldn't be counted or logged as a cache miss - it's a hit on a negative entry. i think we should move this check above of this 'flags' block, ie:

   ObjectCacheInfo& src = iter->second.info;
+  if (src.status == -ENOENT) {
+    ldout(cct, 10) << "cache get: name=" << name << " : hit (negative entry)" << dendl;
+    if (perfcounter) perfcounter->inc(l_rgw_cache_hit);
+    return -ENODATA;
+  }
   if ((src.flags & mask) != mask) {

we shouldn't treat any status other than ENOENT as a negative entry - if we got any other error trying to read it, that shouldn't prevent us from trying again

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

add negative cache to the system object

Signed-off-by: Or Friedmann <ofriedma@redhat.com>

Fixes: https://tracker.ceph.com/issues/45816
@ofriedma ofriedma requested a review from cbodley July 12, 2020 11:35
@ofriedma
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants