Skip to content

Improved handling of URLs to be purged for better performance. - #65

Open
gnotaras wants to merge 2 commits into
pkhamre:masterfrom
gnotaras:enhance-purge-url-pool
Open

Improved handling of URLs to be purged for better performance.#65
gnotaras wants to merge 2 commits into
pkhamre:masterfrom
gnotaras:enhance-purge-url-pool

Conversation

@gnotaras

@gnotaras gnotaras commented Dec 9, 2013

Copy link
Copy Markdown
Contributor

This patch does not change anything else than the handling of the URLs to be purged.

Instead of placing a PURGE/BAN for each URL as it is generated, all URLs are collected in an array. After removing duplicate URLs from the array, a PURGE/BAN is then placed for each URL. Using this method, we avoid sending duplicate PURGE/BAN requests to the varnish servers and improve performance.

Also, I had to fix the indentation of the WPVarnishPurgePool() function to make it easier for me to set up the loop correctly avoiding unnecessary duplicate collection of the varnish server information or other URL calculations.

Finally, I added changelog entries under the dev release. Please change this accordingly.

Thanks in advance.

Instead of placing a PURGE/BAN for each URL as it is generated, all
URLs are collected in an array. After removing duplicate URLs from
the array, a PURGE/BAN is then placed for each URL. Using this
method, we avoid sending duplicate PURGE/BAN requests to the
varnish servers and improve performance.

This idea was taken from a derivative work (apparently) of
wp-varnish: https://github.com/admingeekz/varnish-wordpress

The code could be further enhanced to send PURGE/BAN requests
using HTTP pipelining to further improve performance.
@gnotaras

Copy link
Copy Markdown
Contributor Author

Hello Pål-Kristian,

I've performed several tests since yesterday and have come to the conclusion that there will always be cases in which (for example) both the publish_post and transition_post_status actions will be triggered resulting in sending duplicate BAN requests to the varnish proxies.

The only way to avoid this is by collecting all URLs to be purged in an array and remove the duplicate ones.

There is also another problem with the current implementation. It processes the varnish backend settings and other options every single time a URL is purged.

This patch resolves all these issues and also makes it easier to further enhance performance by implementing HTTP pipelining in the future.

BTW, thanks for accepting changes to this project.

@gnotaras

Copy link
Copy Markdown
Contributor Author

Hello Pål-Kristian,

I'd definitely appreciate some better communication.

Sending you patches that can be merged cleanly adds much overhead. I am currently working on the admin bar menu because it is incomplete and buggy as well. I really need to know what's going to happen to this, so I know on what base changeset I should work.

Your thoughts on this are very welcome and much appreciated.

@pkhamre

pkhamre commented Dec 11, 2013

Copy link
Copy Markdown
Owner

Hi George,

The last year I have not been contributing any to this project, and I've been thinking to create a GitHub organization that can maintain this project. Then all developers who contribute can join this project and we can have a better discussion forum there. What do you think about that?

@pothi

pothi commented Dec 11, 2013

Copy link
Copy Markdown
Contributor

+1 for a better discussion forum. While, this plugin is the best plugin for Varnish purge, it still lacks direction and best practices that are incorporated in certain WordPress plugins.

@gnotaras

Copy link
Copy Markdown
Contributor Author

Hi Pål-Kristian,

+1. Sounds like a good idea to me as well. Personally, I have very little spare time because of other obligations. But, I think a github organization could help to further improve this plugin in a more collaborative way. The plugin currently is good, but there is definitely room for improvement like @pothi said.

I started working on an experimental fork of this plugin, but I would be glad to contribute any changes to this project.

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.

3 participants