Skip to content

getItemsByTag() - empty after one item has expired #893

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

Closed
1 task done
MaxChri opened this issue Feb 9, 2023 · 10 comments
Closed
1 task done

getItemsByTag() - empty after one item has expired #893

MaxChri opened this issue Feb 9, 2023 · 10 comments

Comments

@MaxChri
Copy link

MaxChri commented Feb 9, 2023

What type of issue is this?

Incorrect/unexpected/unexplainable behavior

Operating system + version

Debian 11

PHP version

8.1

Connector/Database version (if applicable)

Memcached, Files

Phpfastcache version

9.1.2 ✅

Describe the issue you're facing

I create multiple keys with different expiration values using expiresAfter(). I'm using the addTags() function to search for the keys if necessary (e.g to manage them in a CMS). When a first item key expires, all other keys with the same tag name are immediately gone and are not callable anymore with getItemsByTag().

So basically if you have a 5 seconds key and another 10 seconds key (both initialized at the same time) with the same tag name, both items will be returned by getItemsByTag(). But if the 5 seconds key has expired, the 10 seconds key is also gone when calling getItemsByTag(). The 10 seconds key is still physically there but cannot be found by the previously declared tag anymore. So a 10 seconds timer would be gone within 5 seconds in this scenario. If the 5 seconds timer would not exist, the 10 seconds timer would of course counting down until 0.

Expected behavior

All keys with the same tag name should be still callable, even if previous keys with the same tag has been expired.

Code sample (optional)

No response

Suggestion to fix the issue (optional)

No response

References (optional)

No response

Do you have anything more you want to share? (optional)

In the video you can see, that when the statistics key expires, the player object key is also gone while it should have 5 seconds left. At the same time, a new statistics key will be created (which is not important).
https://i.gyazo.com/ad584c00607032c41a83db2378ff51d8.mp4

Have you searched in our Uncyclo before posting ?

  • I have searched over the Uncyclo
@github-actions
Copy link

github-actions bot commented Feb 9, 2023

Hello curious contributor !
Since it seems to be your first contribution, make sure that you've been:

  • Reading and searching out our WIKI
  • Reading and agreed with our Code Of Conduct
  • Reading and understood our Coding Guideline
  • Reading our README
    If everything looks unclear to you, tell us what 😄
    The Phpfastcache Team

@Geolim4
Copy link
Member

Geolim4 commented Feb 9, 2023

Hello,

I'm not sure how you get this behavior since the cache already benefits from the cache item with the longest date:
https://github.com/PHPSocialNetwork/phpfastcache/blob/master/lib/Phpfastcache/Core/Pool/TaggableCacheItemPoolTrait.php#L430

Also the tags are tested against this behavior too:
https://github.com/PHPSocialNetwork/phpfastcache/blob/master/tests/ItemTags.test.php

@MaxChri
Copy link
Author

MaxChri commented Feb 9, 2023

Hello,

I'm not sure how you get this behavior since the cache already benefits from the cache item with the longest date: https://github.com/PHPSocialNetwork/phpfastcache/blob/master/lib/Phpfastcache/Core/Pool/TaggableCacheItemPoolTrait.php#L430

Also the tags are tested against this behavior too: https://github.com/PHPSocialNetwork/phpfastcache/blob/master/tests/ItemTags.test.php

Hey Geo,

If you have multiple keys which have the same tag name and one key gets expired, all keys with the same tag name will be not be displayed anymore (even if they have been not expired) when using getItemsByTag(). This bug doesn't occour if the keys have different tag names. It also doesn't matter if you add just one tag or multiple. As long as one tag gets expired, all other keys with the same tags are gone as well.

In the code example you can see that key_1 getting removed at the same time of key_2, because key_2 getting expired after 5 seconds, while key_1 should have still 5 seconds to live. The cache item key_1 is still there in the pool but cannot longer be called with getItemsByTag().

Simple test:

// instance
$InstanceCache = CacheManager::getInstance('memcached', new Config([
	'host' =>'127.0.0.1',
	'port' => 11211,
	// 'sasl_user' => false, // optional
	// 'sasl_password' => false // optional
]));

// key_1
$CachedString1 = $InstanceCache->getItem("key_1");
if (is_null($CachedString1->get())) {
	$CachedString1->set("data1")->expiresAfter(10);
	$CachedString1->addTag("query");
	$InstanceCache->save($CachedString1);
}

// key_2
$CachedString2 = $InstanceCache->getItem("key_2");
if (is_null($CachedString2->get())) {
	$CachedString2->set("data2")->expiresAfter(5);
	$CachedString2->addTag("query");
	$InstanceCache->save($CachedString2);
}

// application layer
$keys = $InstanceCache->getItemsByTag("query");
$aCacheObjects = null;
foreach ($keys as $key) {
	$oCacheItem = new stdClass();
	$oCacheItem->key = $key->getKey();
	$oCacheItem->ttl = $key->getTtl();
	$oCacheItem->tags = $key->getTags();
	$aCacheObjects[] = $oCacheItem;
}

// render
echo "<pre>";
print_r($aCacheObjects);
echo "</pre>";

@Geolim4
Copy link
Member

Geolim4 commented Feb 9, 2023

Interesting, if you set the 5 sec key first then the 10 sec, the result are valid.
By your example, you does the opposite and there's, indeed an issue.

@Geolim4
Copy link
Member

Geolim4 commented Feb 9, 2023

That's why I doubted it first, because my tests are doing the first use case, not yours. I'm investigating.

@Geolim4
Copy link
Member

Geolim4 commented Feb 9, 2023

Can you please open lib/Phpfastcache/Core/Pool/TaggableCacheItemPoolTrait.php

Search for:

            $tagsItem->set(\array_merge((array)$data, [$item->getKey() => $expTimestamp]))
                ->expiresAt($item->getExpirationDate());

and replace it with:

            $tagsItem->set(\array_merge((array)$data, [$item->getKey() => $expTimestamp]));

            if (!$tagsItem->isHit() || $tagsItem->getExpirationDate() < $item->getExpirationDate()) {
                $tagsItem->expiresAt($item->getExpirationDate());
            }

and tell me if it's working on your side :)

@MaxChri
Copy link
Author

MaxChri commented Feb 9, 2023

I have replaced the code and it works perfectly now. Thank you very much for your quick help, I really appreciate it! :)

@Geolim4
Copy link
Member

Geolim4 commented Feb 9, 2023 via email

@Geolim4
Copy link
Member

Geolim4 commented Feb 10, 2023

I will make an anti-regression test as well, since this behavior was difficult to catch without your specific case.

@Geolim4
Copy link
Member

Geolim4 commented Feb 11, 2023

Versions 9.1.3 and 8.1.4 have been released.

Happy caching !

Geolim4 added a commit that referenced this issue Feb 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants