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

[core] Implemented MemcachedCache #1000

Merged
merged 11 commits into from
May 3, 2019
Merged

Conversation

em92
Copy link
Contributor

@em92 em92 commented Jan 7, 2019

In my bridge instance to reduce usage of file system, MemcachedCache was implemented to use memcached server as feed items cache.

To turn it on I made hardcode edit:

diff --git a/index.php b/index.php
index a95302a..7c78f83 100644
--- a/index.php
+++ b/index.php
@@ -202,7 +202,7 @@ try {
                );
 
                // Initialize cache
-               $cache = Cache::create('FileCache');
+               $cache = Cache::create('MemcachedCache');
                $cache->setPath(PATH_CACHE);
                $cache->purgeCache(86400); // 24 hours
                $cache->setParameters($cache_params);

It would be cool, if user could use some other cache just by editing config. But in current implementation, there is no such config. So for this, we need next config items:

  • which cache to use (FileCache or MemcachedCache)
  • params for caches themselves (directory or list for servers)

Here we discuss, how to implement MemcachedCache to bridge instance in normal way

@logmanoriginal logmanoriginal added New-Feature This is a new feature Code-Investigation-Needed Code needs to be investigated labels Jan 7, 2019
@logmanoriginal
Copy link
Contributor

Nice work 🥇

Just to be sure, Memcached is some sort of external service?

To turn it on I made hardcode edit

It would be cool, if user could use some other cache just by editing config.

Good point, this clearly needs to be fixed. I've added the Code-Investigation-Needed label for that reason.

to reduce usage of file system

Out of curiosity, would compressing files do any good?

@em92
Copy link
Contributor Author

em92 commented Jan 8, 2019

Memcached is some sort of external service?

Yes. https://memcached.org/

Out of curiosity, would compressing files do any good?

It will slow down bridge instance itself 'cos of decompressing and compressing. Feed items cached data in my instance was less than 5 mb, AFAIK

@logmanoriginal
Copy link
Contributor

I'm gonna merge #1002 in a minute, which breaks this PR because the code from index.php moves to /actions/DisplayAction.php#L88.

Would it be okay for you to merge this PR without changes to index.php? (assuming it is stable)
The cache type needs a configuration option, which is a separate topic anyway.

@em92
Copy link
Contributor Author

em92 commented Feb 6, 2019

No problem. I will fix this PR

@logmanoriginal
Copy link
Contributor

Thanks, support for custom cache names will follow in a minute...

logmanoriginal added a commit that referenced this pull request Feb 6, 2019
This commit adds support for a new parameter which specifies the type
of cache to use for caching. It is specified in config.ini.php:

 [cache]

 type = "..."

Currently only one type of cache is supported (see /caches). All uses
of 'FileCache' were replaced by this configuration option.

Note: Caching currently depends on files and folders (due to FileCache).
Experience may vary depending on the selected cache type. For now always
check if FileCache is working before testing alternative types.

References #1000
@logmanoriginal
Copy link
Contributor

Adding

; Defines the cache type used by RSS-Bridge
; "file" = FileCache (default)
; "memcache" = MemcacheCache
type = "memcache"

to config.ini.php should now work (actually might not work because it ends on 'cache' which is filtered by lib/Cache.php!?).

@em92
Copy link
Contributor Author

em92 commented Feb 7, 2019

actually might not work because it ends on 'cache' which is filtered by lib/Cache.php!?

Fixed with:

diff --git a/lib/Cache.php b/lib/Cache.php
index 6826d4c..6c2943a 100644
--- a/lib/Cache.php
+++ b/lib/Cache.php
@@ -193,7 +193,7 @@ class Cache {
                        }
 
                        // Trim trailing 'Cache' if exists
-                       if(preg_match('/(.+)(?:Cache)/i', $name, $matches)) {
+                       if(preg_match('/(.+)(?:Cache)$/i', $name, $matches)) {
                                $name = $matches[1];
                        }
 

@logmanoriginal
Copy link
Contributor

I've added your fix to master. Hope it works now. Regarding Travis, here is a rather new discussion regarding memcached support which could be of help: https://travis-ci.community/t/unable-to-load-dynamic-library-memcached-so/2232

@logmanoriginal
Copy link
Contributor

Can this be merged?

@logmanoriginal
Copy link
Contributor

Please test if this still works on current master. #1060 introduced a few changes.

@em92
Copy link
Contributor Author

em92 commented Apr 30, 2019

Please test if this still works on current master

It does not. I will make changes for it to work.

@em92
Copy link
Contributor Author

em92 commented Apr 30, 2019

Looks fine on first minutes on my instance. I will comment here tomorrow morning (YEKT) if everything is fine.

@em92 em92 changed the title WIP: [core] Implemented MemcachedCache [core] Implemented MemcachedCache May 3, 2019
@em92
Copy link
Contributor Author

em92 commented May 3, 2019

Works fine. It can be merged now

@teromene teromene merged commit 75359bc into RSS-Bridge:master May 3, 2019
@teromene
Copy link
Member

teromene commented May 3, 2019

Thanks for the hard work !

@em92 em92 deleted the memcached-cached branch May 3, 2019 14:26
infominer33 pushed a commit to web-work-tools/rss-bridge that referenced this pull request Apr 17, 2020
This commit adds support for a new parameter which specifies the type
of cache to use for caching. It is specified in config.ini.php:

 [cache]

 type = "..."

Currently only one type of cache is supported (see /caches). All uses
of 'FileCache' were replaced by this configuration option.

Note: Caching currently depends on files and folders (due to FileCache).
Experience may vary depending on the selected cache type. For now always
check if FileCache is working before testing alternative types.

References RSS-Bridge#1000
infominer33 pushed a commit to web-work-tools/rss-bridge that referenced this pull request Apr 17, 2020
infominer33 pushed a commit to web-work-tools/rss-bridge that referenced this pull request Apr 17, 2020
* [core] Implemented MemcachedCache
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code-Investigation-Needed Code needs to be investigated New-Feature This is a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants