Skip to content

Commit

Permalink
Fix metrics when delete non existing page
Browse files Browse the repository at this point in the history
### What changes are proposed in this pull request?

Fix metrics when delete non existing page

### Why are the changes needed?
When deleting non-existing page we see a abnormal spike in PageAge. Still doesn't know why we have abnormal PageAge but since it's non-existing page we would not able to read it as well so it make sense to not record the age.
Also we continue to delete page from page store if we didn't find it in metastore so we avoid inconsistency between pagestore and metastore

### Does this PR introduce any user facing changes?
na

			pr-link: #18618
			change-id: cid-7d4803cb2d8c433d3ad077a7bf9e3074d91b3389
  • Loading branch information
jja725 authored and alluxio-bot committed Jun 7, 2024
1 parent 8f48a25 commit 9bca673
Showing 1 changed file with 45 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -751,6 +751,50 @@ public boolean append(PageId pageId, int appendAt, byte[] page, CacheContext cac
return put(pageId, page, cacheContext);
}

/**
* delete the specified page. Be cautious that this method will return true if the page
* does not exist since the page already gone.
*
* @param info pageInfo
* @param isTemporary whether is it temporary or not
* @return whether the page is deleted successfully or not
*/
public boolean deletePageIfExists(PageInfo info, boolean isTemporary) {
if (mState.get() != READ_WRITE) {
Metrics.DELETE_NOT_READY_ERRORS.inc();
Metrics.DELETE_ERRORS.inc();
return false;
}
boolean ok = true;
ReadWriteLock pageLock = getPageLock(info.getPageId());
try (LockResource r = new LockResource(pageLock.writeLock())) {
try (LockResource r1 = new LockResource(mPageMetaStore.getLock().writeLock())) {
try {
mPageMetaStore.removePage(info.getPageId(), isTemporary);
} catch (PageNotFoundException e) {
Metrics.DELETE_NON_EXISTING_PAGE_ERRORS.inc();
Metrics.DELETE_ERRORS.inc();
// pass through to delete the page from page store
}
}
try {
info.getLocalCacheDir().getPageStore().delete(info.getPageId(), isTemporary);
} catch (IOException e) {
LOG.error("Failed to delete page {} (isTemporary: {}) from pageStore.",
info.getPageId(), isTemporary, e);
ok = false;
Metrics.DELETE_STORE_DELETE_ERRORS.inc();
Metrics.DELETE_ERRORS.inc();
} catch (PageNotFoundException e) {
Metrics.DELETE_NON_EXISTING_PAGE_ERRORS.inc();
Metrics.DELETE_ERRORS.inc();
ok = true;
}
LOG.debug("delete({}) exits, success: {}", info.getPageId(), ok);
return ok;
}
}

/**
* Restores a page store at the configured location, updating meta store accordingly.
* If restore process fails, cleanup the location and create a new page store.
Expand Down Expand Up @@ -921,7 +965,7 @@ public void invalidate(Predicate<PageInfo> predicate) {
PageInfo pageInfo = pageInfoOpt.get();
boolean isPageDeleted = false;
if (predicate.test(pageInfo)) {
isPageDeleted = delete(pageInfo.getPageId());
isPageDeleted = deletePageIfExists(pageInfo, false);
}
if (isPageDeleted) {
MetricsSystem.meter(MetricKey.CLIENT_CACHE_PAGES_INVALIDATED.getName()).mark();
Expand Down

0 comments on commit 9bca673

Please sign in to comment.