----- Start Original Message ----- Sent: Wed, 29 Aug 2007 12:10:26 -0400 From: "Moshe Weitzman" <weitzman@tejasa.com> To: development@drupal.org Subject: Re: [development] node caching

1. your data must now be dynamic after load. you make changes in nodeapi('view') operation for example. if this causes pain, please give a use case that can't be reworked to play nice with a node laod cache.

Or roll back the whole thing and move on.

Karoly: are YOU arguing that it should be rolled back?

Never. See my latest comment in the issue http://drupal.org/node/111127 If someone would have read the issue (I just counted it) the fourth line reads: "poll.module needs to be patched to do user specific stuff in hook_view not hook_load" Sometime later: "it was doing logic in load which belongs to view. Outdated little module, poll is." Is this enough to give you some hints on your questions? Seems I need to write the update docs as well. But it's pretty apparent from the issue. (Which people apparently have not reviewed so there.)

I don't want to spend much time discussing the node caching issue, as I've already weighed in. But there have been a number of responses that suggested I need to produce a list of 'broken modules' or suggestions that I'm FUDding the addition of caching. I'd like to explain in a bit of detail what my concerns are; if they're solved, so be it. If not, well, same deal. I just want to make sure that these issues are raised. There are a number of modules that it will definitely complicate things for (they'll have to learn a new way of managing data to avoid strange unpredictable 'stale data' bugs, and it ISN'T simply a matter of moving to view() operations on any sufficiently complex system). Obviously, developers can simply be told, "change your code, it's different now." My biggest concerns and frustrations are based on two things: 1) Judging from the issue itself, it went in without considering the impact on sites where high interactive traffic (voting on nodes, etc) would frequently reset cached nodes. In these situations, caching could easily make things WORSE, as nodes will rapidly be loaded, cached and cleared rather than simply loaded. The use of a 'nocache' param is certainly one solution, but it's an after-the-fact fix; it becomes the new, subtle, strange version of advanced caching's hook_init warning. "If you see something strange, maybe it's node caching." 2) It breaks existing APIs more than a month after the code freeze, __in a way that is NOT required to fix a critical bug__. It enhances performance in many cases, yes. But so would many API-breaking changes. I would have no problem with simply telling contrib authors to move their code to another op __if we were not past freeze.__ I've even argued for the addition of __non-breaking__ functionality post- freeze. But stopping breaking changes from coming in at the last minute is PRECISELY what the freeze is supposed to do. Do I think that the feature is fundamentally flawed? NO. Do I think that it went in without sufficient consideration of the varied activity footprints of high-traffic production sites? YES. I've said my piece, and I don't plan on dwelling any more on the issue. If others feel it's important (Morbus and Crell are two who've spoken up with concerns) they can do so. There are other important things to work on and I don't want to end up spending my cycles going back and forth on this particular one. If Dries and Goba decide to keep it and the concerns of others affected by it are quelled, so be it. I do think, though, that our very loose definition of code freeze (no breaking API changes unless required to fix a critical bug) should be revised. It should mean SOMETHING other than 'we think we're probably done breaking things, unless we come up with something cool.' --Jeff

On 8/30/07, Dries Buytaert <dries.buytaert@gmail.com> wrote:

On 30 Aug 2007, at 08:45, Jeff Eaton wrote:

...

If Dries and Goba decide to keep it and the concerns of others affected by it are quelled, so be it. I do think, though, that our very loose definition of code freeze (no breaking API changes unless required to fix a critical bug) should be revised. It should mean SOMETHING other than 'we think we're probably done breaking things, unless we come up with something cool.'

I'd actually prefer to rollback that patch, and to work on it more during the Drupal 7 release cycle. -- Dries Buytaert :: http://www.buytaert.net/

Rolled back the patch. Feel free to submit follow up patches to http://drupal.org/node/111127 , if you find something was not properly rolled back. Gabor

I run node caching on several sites for Drupal 5 with the advcache module. It is and always will be available. Even for Drupal 6. Robin Monks wrote:

On 8/30/07, *Gábor Hojtsy* <gabor@hojtsy.hu <mailto:gabor@hojtsy.hu>> wrote:

Rolled back the patch.

NOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOoooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo !

No another year until we have none caching...please!!!!

+100 on this. I believe this to be a worthy cause. I have been feverishly working on the APC module (d.o/project/apc) to get it ready for D6 to have an out of the box single server non-db caching option (that doesn't require running memcached processes). I really believe that the work that RD has done on the advcache stuff is _GREAT_ since I have personally seen it running in the "real world" and would be much easier to implement with just being able to turn the module on much the same way you can do with the pluggable caching. Without this functionality, we will never get to test that code sufficiently at the magnitude we need to in order to have a really solid caching solution for nodes. Please, please, please let's consider adding this hook or at least keep the patch and make it a configurable option that is turned off by default. Maybe it is just too late in the game now. Steve Rude Jeff Eaton wrote:

I would actually love a simple hook in the node load code, not unlike custom_url_rewrite(), that allows modules like advcache to work directly as part of the node loading mechanism. That would be three, maybe four lines. I just feel strongly that making a breaking change 'opt out' rather than an 'opt in' this late in the cycle, without a lot of large-scale testing, is dangerous.

Robert says that we can already do this, with some effort. Those making really huge Drupal sites should really be prepared to put some effort in. Drupal will rarely fall out of my downloads folder and onto a production server. Save patches like these for D7 and move on.

What was the compelling reason for this to go in post freeze?

On the last project that I worked with Robert on, the advcache and memcache projects were used. It was a very complex site, and thus the caching solution was fairly complex as far as that goes. As Drupal becomes the standard for large dynamic media websites, any place that we can add performance is going to allow Drupal to be used by even more large complex sites and thus the entire project will continue to expand and get more large commercial companies funding the growth which is good for everyone. If we have a technology like node caching that is only used on 3 major Drupal sites via patches, with each implementation custom or slightly different, it will certainly not be any more stable for Drupal 7 than it is now. If we have hundreds or thousands of people who just have to click a radio button or add an option to settings.php to turn on node caching and label it as experimental, then we can begin to stabilize this great performance boosting technology. If it doesn't work with your particular set of modules, fine, turn it off and no harm done. I believe that since nodes are the crux of the Drupal system, releasing Drupal 6 without this technology is a real shame since there are so many great performance options that will be available for Drupal 6. Steve Rude

sean - thats basically what chx' lats patch proposed. caching was skipped if a variable was present. a contrib module could manage setting of that var. the patch was denied in favor of rollback. On 8/30/07, Sean Robertson <seanr@ngpsoftware.com> wrote:

...

If we have a technology like node caching that is only used on 3 major Drupal sites via patches, with each implementation custom or slightly different, it will certainly not be any more stable for Drupal 7 than it is now. If we have hundreds or thousands of people who just have to click a radio button or add an option to settings.php to turn on node caching and label it as experimental, then we can begin to stabilize this great performance boosting technology. If it doesn't work with your particular set of modules, fine, turn it off and no harm done.

I believe that since nodes are the crux of the Drupal system, releasing Drupal 6 without this technology is a real shame since there are so many great performance options that will be available for Drupal 6.

I agree with this. How hard would it be to make it an option, just like the advanced cache option in Drupal 5 and warn users that it could cause issues in some circumstances.

-- Sean Robertson Web Developer NGP Software, Inc. seanr@ngpsoftware.com (202) 686-9330 http://www.ngpsoftware.com

With all due respect, Robin, this doesn't help. Technical issues never come down to a vote. They come down what's demonstrably the best solution. Further more, if you remember the pain that we experienced releasing 4.7 (last minute FAPI), you'll probably understand why many people want a code freeze to really mean something. I'm in favor of node caching. I'm also supportive of the decision to roll it back. -Robert Robin Monks wrote:

Would demanding a vote help matters?

If so....

I demand a vote!

If not...

Robin

On Wednesday 29 August 2007, Moshe Weitzman wrote:

1. your data must now be dynamic after load. you make changes in nodeapi('view') operation for example. if this causes pain, please give a use case that can't be reworked to play nice with a node laod cache.

I'm finishing up a site right now where, for various reasons, the majority of my node_load() calls are not then followed by a view op. They're loaded for their data, and only sometimes is the node that is loaded the currently viewed node. There's frequently at least three per page that are not. While I'd love to make node_load() less expensive, I really don't think a change of this size should be happening nearly 2 months after an extended freeze. -- Larry Garfield AIM: LOLG42 larry@garfieldtech.com ICQ: 6817012 "If nature has made any one thing less susceptible than all others of exclusive property, it is the action of the thinking power called an idea, which an individual may exclusively possess as long as he keeps it to himself; but the moment it is divulged, it forces itself into the possession of every one, and the receiver cannot dispossess himself of it." -- Thomas Jefferson