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

APCng collect - do not throw exception if a metakey is missing #151

Closed
wants to merge 1 commit into from

Conversation

ashandi
Copy link

@ashandi ashandi commented Apr 19, 2024

These changes should fix the following issues:
#124
#126

The main idea of this PL is following: when we use APCng with ttl 0 we have no guaranty that our data will not be removed from the cache. So the case when we can't find some metakey in the cache is common and we shouldn't rise the exception that stops completely the metrics collection process.

The same solution is already implemented in the Redis Storage:

if (!isset($raw['__meta'])) {
continue;
}

$metaKey = $this->removePrefixFromKey($metaKeyWithPrefix);
$rawSummary = $this->redis->get($metaKey);
if ($rawSummary === false) {
continue;
}

if (!isset($raw['__meta'])) {
continue;
}

if (!isset($raw['__meta'])) {
continue;
}

No meta data - no statistics.

Another problem here is that the metadata stored only once for a metric. So if we loose metadata from the cache, the metric will continue collecting but we lost access to it. So I made calling of storeMetadata function always, not only on the first writing of the metric. This function checks inside that metadata is already exists in the cache and will not do anything if it is.

@ashandi
Copy link
Author

ashandi commented May 9, 2024

According to this, this exception shouldn't be caught if the apc.shm_size setting has enough value.

@ashandi ashandi closed this May 9, 2024
@ashandi ashandi deleted the apcng_collect_fix_exceptions branch May 9, 2024 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants