You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by "Pan Jie (JIRA)" <ji...@apache.org> on 2009/02/25 09:41:01 UTC
[jira] Created: (SHINDIG-943) add support for 0.9 limited
invalidation
add support for 0.9 limited invalidation
----------------------------------------
Key: SHINDIG-943
URL: https://issues.apache.org/jira/browse/SHINDIG-943
Project: Shindig
Issue Type: New Feature
Components: PHP
Reporter: Pan Jie
http://wiki.opensocial.org/index.php?title=Limited_Invalidation
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
Re: [jira] Commented: (SHINDIG-943) add support for 0.9 limited
invalidation
Posted by Chris Chabot <ch...@google.com>.
Cool, I wasn't sure if the patch from the code review was on top of- or
instead of the 2 patches in the jira issue. I'll go by the codereview one
then. Thanks!
On Thu, Mar 5, 2009 at 12:30 PM, Pan Jie (JIRA) <ji...@apache.org> wrote:
>
> [
> https://issues.apache.org/jira/browse/SHINDIG-943?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12679161#action_12679161]
>
> Pan Jie commented on SHINDIG-943:
> ---------------------------------
>
> I made a new patch in codereview.appspot.com:
> http://codereview.appspot.com/25066/show
> which was synced with trunk.
>
> Let's deprecate these 2 patch files attached in this issue. :)
>
>
>
>
> --
> Warm Regards,
>
> Pan Jie
> panjie@google.com
>
>
> > add support for 0.9 limited invalidation
> > ----------------------------------------
> >
> > Key: SHINDIG-943
> > URL: https://issues.apache.org/jira/browse/SHINDIG-943
> > Project: Shindig
> > Issue Type: New Feature
> > Components: PHP
> > Reporter: Pan Jie
> > Attachments: 20090225.patch, 20090226.patch
> >
> >
> > http://wiki.opensocial.org/index.php?title=Limited_Invalidation
>
> --
> This message is automatically generated by JIRA.
> -
> You can reply to this email to add a comment to the issue online.
>
>
[jira] Resolved: (SHINDIG-943) add support for 0.9 limited
invalidation
Posted by "Chris Chabot (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-943?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Chris Chabot resolved SHINDIG-943.
----------------------------------
Resolution: Fixed
Assignee: Chris Chabot
this has been completed some time ago, closing issue
> add support for 0.9 limited invalidation
> ----------------------------------------
>
> Key: SHINDIG-943
> URL: https://issues.apache.org/jira/browse/SHINDIG-943
> Project: Shindig
> Issue Type: New Feature
> Components: PHP
> Reporter: Pan Jie
> Assignee: Chris Chabot
> Attachments: 20090225.patch, 20090226.patch
>
>
> http://wiki.opensocial.org/index.php?title=Limited_Invalidation
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-943) add support for 0.9 limited
invalidation
Posted by "Chris Chabot (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-943?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12679868#action_12679868 ]
Chris Chabot commented on SHINDIG-943:
--------------------------------------
I've committed the latest patch (set 2) from http://codereview.appspot.com/25066/show
Since it's a rather large patch I'm eager to get it into svn, however there are a few small issues, can you please submit a separate patch for those?
Since codereview tool is giving me a "500 Server Error" when i try to post it, I'll inline my comments here:
Cache.php:
- abstract function delete($key);
+ private $storage = null;
(Draft) 2009/03/07 11:33:10 We usually put all var declarations at the top of
We usually put all var declarations at the top of the class definition and not between functions
+ private function __construct($type, $name, $time) {
+ if ($type == 'CacheFile') {
(Draft) 2009/03/07 11:45:51 This breaks the internal contract of how the Confi
This breaks the internal contract of how the Config values == Class names, in this case that's rather important since large sites pretty much always create / re-use their internal custom cache classes and not our provided ones. In this situation someone would set 'data_cache' => 'FooCache', things would break without really having any contextual or documentation clues to *why* that doesn't work for the cache classes. Better solution would be to edit the default config (& add a comment explaining the change) and put the real class name there, and instance the cache class using: $this->storage = new $cacheClass($name); That way the config class name == class assumption is true again, and people can use their custom classes again
+ $data = unserialize($data);
+ return array('found' => true, 'time' => $data['time'],
(Draft) 2009/03/07 11:38:57 It would be simpler to do: return array_merge(arra
It would be simpler to do: return array_merge(array('found' => true), $data); (arg1 overwrites arg2 if they have the same keys btw, see http://www.php.net/array_merge)
CacheStorage.php:
+abstract class CacheStorage {
(Draft) 2009/03/07 11:49:42 It would be better to force this class to be a sin
It would be better to force this class to be a singleton, especially in cases where the cache backend makes a connection (like the frequently used memcache based implementations), this code would cause multiple connections per request, which slows it down and hogs resources. If you would make the constructor private, have a private $instance var, and a static function for getInstance() (which either returns the already instanced class or creates it if no instance was made yet), and change the code in the Cache class to instance the storage objects using $cacheClass::getInstance($name).
CacheStorageApc.php :
+ private function storageKey($key) {
+ return $prefix . '_' . $key;
(Draft) 2009/03/07 12:16:57 typo, should be $this->prefix
typo, should be $this->prefix
CacheStorageMemcache.php:
+ private function storageKey($key) {
+ return $prefix . '_' . $key;
(Draft) 2009/03/07 12:17:34 typo, should be $this->prefix
typo, should be $this->prefix
+ public function __construct($name) {
+ $this->prefix = $name;
+ if (!self::$memcache) {
(Draft) 2009/03/07 11:56:41 If we'd make the base CacheStorage class a singlet
If we'd make the base CacheStorage class a singleton, we wouldn't need this hack'ish solution anymore
> add support for 0.9 limited invalidation
> ----------------------------------------
>
> Key: SHINDIG-943
> URL: https://issues.apache.org/jira/browse/SHINDIG-943
> Project: Shindig
> Issue Type: New Feature
> Components: PHP
> Reporter: Pan Jie
> Attachments: 20090225.patch, 20090226.patch
>
>
> http://wiki.opensocial.org/index.php?title=Limited_Invalidation
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (SHINDIG-943) add support for 0.9 limited
invalidation
Posted by "Pan Jie (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-943?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Pan Jie updated SHINDIG-943:
----------------------------
Attachment: 20090226.patch
Subtask 2 Add invalidatation for BasicRemoteContent finished.
I also enrich unittest for BasicRemoteContent class.
> add support for 0.9 limited invalidation
> ----------------------------------------
>
> Key: SHINDIG-943
> URL: https://issues.apache.org/jira/browse/SHINDIG-943
> Project: Shindig
> Issue Type: New Feature
> Components: PHP
> Reporter: Pan Jie
> Attachments: 20090225.patch, 20090226.patch
>
>
> http://wiki.opensocial.org/index.php?title=Limited_Invalidation
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-943) add support for 0.9 limited
invalidation
Posted by "Pan Jie (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-943?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12676608#action_12676608 ]
Pan Jie commented on SHINDIG-943:
---------------------------------
About Limited invalidation, here're my thoughts:
1. Caching happened in 2 places: stuff needed by gadget rendering(including gadget specs, gadget message bundles), and stuff fetched by makeRequest.
2. All these stuff are cached when fetching, aka, BasicRemoteContent->fetch().
3. We need to support 2 forms of invalidation-key: url and opensocial id.
Supporting invalidating url is relatively easy, cause key in cache is RemoteContentRequest.
Supporting invalidating opensocial id means that we should invalidate all stuff fetched by signed makeRequest related to a specified user. To support this, we need to store these stuff into a new cachewhose key is user id and url.
4. If fetching failed, we can use cached version even if it's expired.
Because we need expired values to have higher availability, CacheApc and CacheMemcache need to store time/ttl just like CacheFile. I refactored those cache in 20090225.patch.
> add support for 0.9 limited invalidation
> ----------------------------------------
>
> Key: SHINDIG-943
> URL: https://issues.apache.org/jira/browse/SHINDIG-943
> Project: Shindig
> Issue Type: New Feature
> Components: PHP
> Reporter: Pan Jie
> Attachments: 20090225.patch
>
>
> http://wiki.opensocial.org/index.php?title=Limited_Invalidation
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-943) add support for 0.9 limited
invalidation
Posted by "Chris Chabot (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-943?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12679156#action_12679156 ]
Chris Chabot commented on SHINDIG-943:
--------------------------------------
Pan I don't suppose you could take a quick look into making the 20090226 patch apply against the trunk? Right now it fails to apply on hp/src/common/sample/BasicRemoteContent.php. Initial review of the code looked good, and I'll take a bit more thorough look once I can get this fully up and running locally
> add support for 0.9 limited invalidation
> ----------------------------------------
>
> Key: SHINDIG-943
> URL: https://issues.apache.org/jira/browse/SHINDIG-943
> Project: Shindig
> Issue Type: New Feature
> Components: PHP
> Reporter: Pan Jie
> Attachments: 20090225.patch, 20090226.patch
>
>
> http://wiki.opensocial.org/index.php?title=Limited_Invalidation
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-943) add support for 0.9 limited
invalidation
Posted by "Pan Jie (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-943?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12679161#action_12679161 ]
Pan Jie commented on SHINDIG-943:
---------------------------------
I made a new patch in codereview.appspot.com:
http://codereview.appspot.com/25066/show
which was synced with trunk.
Let's deprecate these 2 patch files attached in this issue. :)
--
Warm Regards,
Pan Jie
panjie@google.com
> add support for 0.9 limited invalidation
> ----------------------------------------
>
> Key: SHINDIG-943
> URL: https://issues.apache.org/jira/browse/SHINDIG-943
> Project: Shindig
> Issue Type: New Feature
> Components: PHP
> Reporter: Pan Jie
> Attachments: 20090225.patch, 20090226.patch
>
>
> http://wiki.opensocial.org/index.php?title=Limited_Invalidation
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-943) add support for 0.9 limited
invalidation
Posted by "Pan Jie (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-943?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12680081#action_12680081 ]
Pan Jie commented on SHINDIG-943:
---------------------------------
Yes, I have a local patch for partuza. I will send code review for partuza
soon.
--
Warm Regards,
Pan Jie
panjie@google.com
> add support for 0.9 limited invalidation
> ----------------------------------------
>
> Key: SHINDIG-943
> URL: https://issues.apache.org/jira/browse/SHINDIG-943
> Project: Shindig
> Issue Type: New Feature
> Components: PHP
> Reporter: Pan Jie
> Attachments: 20090225.patch, 20090226.patch
>
>
> http://wiki.opensocial.org/index.php?title=Limited_Invalidation
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (SHINDIG-943) add support for 0.9 limited
invalidation
Posted by "Pan Jie (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-943?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Pan Jie updated SHINDIG-943:
----------------------------
Attachment: 20090225.patch
> add support for 0.9 limited invalidation
> ----------------------------------------
>
> Key: SHINDIG-943
> URL: https://issues.apache.org/jira/browse/SHINDIG-943
> Project: Shindig
> Issue Type: New Feature
> Components: PHP
> Reporter: Pan Jie
> Attachments: 20090225.patch
>
>
> http://wiki.opensocial.org/index.php?title=Limited_Invalidation
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-943) add support for 0.9 limited
invalidation
Posted by "Chris Chabot (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-943?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12679872#action_12679872 ]
Chris Chabot commented on SHINDIG-943:
--------------------------------------
Ps this patch broke Partuza, since it still depends on the old Cache methodigy still, if something breaks partuza I prefer to have patches for both since otherwise it's hard to really test the changes
> add support for 0.9 limited invalidation
> ----------------------------------------
>
> Key: SHINDIG-943
> URL: https://issues.apache.org/jira/browse/SHINDIG-943
> Project: Shindig
> Issue Type: New Feature
> Components: PHP
> Reporter: Pan Jie
> Attachments: 20090225.patch, 20090226.patch
>
>
> http://wiki.opensocial.org/index.php?title=Limited_Invalidation
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-943) add support for 0.9 limited
invalidation
Posted by "Chris Chabot (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-943?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12682275#action_12682275 ]
Chris Chabot commented on SHINDIG-943:
--------------------------------------
Updated patch looked good to me & has been committed, thanks for the hard work!
> add support for 0.9 limited invalidation
> ----------------------------------------
>
> Key: SHINDIG-943
> URL: https://issues.apache.org/jira/browse/SHINDIG-943
> Project: Shindig
> Issue Type: New Feature
> Components: PHP
> Reporter: Pan Jie
> Attachments: 20090225.patch, 20090226.patch
>
>
> http://wiki.opensocial.org/index.php?title=Limited_Invalidation
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.