Skip to content

Shorten long NODE cache tag #364

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

ilnytskyi
Copy link

In cases when menu contains many nodes, it adds tags to 'x-magento-tags header.
Causing errors related to max header size allowed in different services like Varnish or Nodejs

Shortening default tag length will lower the impact.
image

Also can be addressed by increasing header size on Varnish.
Or nodejs, but in some versions it's reported to not working correctly like - nodejs/node#47246

@ilnytskyi
Copy link
Author

Another things to consider.
Do we even need the individual nodes tags?
Can't we just invalidate whole menu cache when one of the nodes changes?

@adamwaclawczyk
Copy link
Member

Hi @ilnytskyi and thanks for your contribution :)

Changes look ok to me and I think it should not impact anything on Luma/Hyva stores.

I checked some stores and I do not see the snowdog_menu_node on any storefront pages - can you share where(e.g. which url) the node cache tags are attached?

Do we even need the individual nodes tags?
Can't we just invalidate whole menu cache when one of the nodes changes?

That's a valid point, I'll raise this internally but feel free to contribute

@adamwaclawczyk adamwaclawczyk self-requested a review May 6, 2025 09:33
@ilnytskyi
Copy link
Author

@adamwaclawczyk the tags are added when menu is loaded via graphlq using GET method.

Probably because of this Identity class
https://github.com/SnowdogApps/magento2-menu/blob/develop/Model/GraphQl/Resolver/Node/Identity.php

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants