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

feature: support for proxy caching plugin based on disk. #1153

Merged
merged 22 commits into from
Mar 15, 2020
Merged

feature: support for proxy caching plugin based on disk. #1153

merged 22 commits into from
Mar 15, 2020

Conversation

agile6v
Copy link
Member

@agile6v agile6v commented Feb 23, 2020

Summary

This is a prototype implementation and there is a lot of work to be done.

Hopefully it does not affect the review. Thanks.

The admin api looks like the following:

{
    "plugins": {
        "proxy-cache": {
           "cache_zone": "disk_cache_one",
           "cache_key": "${uri}-cache-key",
           "cache_bypass": "${arg_bypass}",
           "cache_method": ["GET"],
           "cache_http_status": [200],
           "hide_cache_headers": true,
           "no_cache": "$arg_no_cache"
        }
    },
    "upstream": {
        "nodes": {
            "127.0.0.1:1999": 1
        },
        "type": "roundrobin"
    },
    "uri": "/hello"
}

Fix #1127

@agile6v
Copy link
Member Author

agile6v commented Feb 23, 2020

Currently only supports the upstream dynamically specify the cache time. If the upstream does not specify it such as the header Expires or Cache-Control, the cache_ttl in the config.yaml will be used. And the option hide_cache_headers will be decide whether the upstream header Expires and Cache-Control is sent to the client.
It's just a trick. Hopefully it can be easily understand.
Thanks.

@moonming
Copy link
Member

how to add test cases for this plugin?

@agile6v
Copy link
Member Author

agile6v commented Feb 26, 2020

Hi @moonming

Tests cases I think the t/servroot/conf/nginx.conf file should be modified.

In recent days, i have been preparing to make a presentation. So I'm gonna add tests in this weekend.
And are there any other problems with code implementation?
Thanks.

lua/apisix/plugins/proxy-cache.lua Outdated Show resolved Hide resolved
lua/apisix/plugins/proxy-cache.lua Outdated Show resolved Hide resolved
lua/apisix/plugins/proxy-cache.lua Outdated Show resolved Hide resolved
lua/apisix/plugins/proxy-cache.lua Outdated Show resolved Hide resolved
lua/apisix/plugins/proxy-cache.lua Show resolved Hide resolved
lua/apisix/plugins/proxy-cache.lua Outdated Show resolved Hide resolved
conf/config.yaml Outdated Show resolved Hide resolved
conf/config.yaml Outdated Show resolved Hide resolved
lua/apisix/plugins/proxy-cache.lua Outdated Show resolved Hide resolved
bin/apisix Outdated Show resolved Hide resolved
@membphis
Copy link
Member

membphis commented Mar 2, 2020

How about the new style? I think the new style is simpler.

# current style
"proxy-cache": {
    "cache_key": ["$uri"]
    "cache_bypass": ["$arg_bypass"],
    ... ...
}
# new style
"proxy-cache": {
    "cache_key": ["uri"]
    "cache_bypass": ["arg_bypass"],
    ... ...
}

@membphis
Copy link
Member

membphis commented Mar 2, 2020

And I have other new questions:

  1. need some e2e test case
  2. need a way to purge the old cache

@agile6v
Copy link
Member Author

agile6v commented Mar 2, 2020

How about the new style? I think the new style is simpler.

# current style
"proxy-cache": {
    "cache_key": ["$uri"]
    "cache_bypass": ["$arg_bypass"],
    ... ...
}
# new style
"proxy-cache": {
    "cache_key": ["uri"]
    "cache_bypass": ["arg_bypass"],
    ... ...
}

Here can use variables or strings, so need an identifier to distinguish it.

@agile6v
Copy link
Member Author

agile6v commented Mar 2, 2020

And I have other new questions:

  1. need some e2e test case
  2. need a way to purge the old cache

I'm also considering these things. For 2 if there is no better solution, cache purge script will be the final choice.

@agile6v
Copy link
Member Author

agile6v commented Mar 2, 2020

And I have other new questions:

  1. need some e2e test case
  2. need a way to purge the old cache
  1. e2e test cases I haven't figured it out yet. Do you have any suggestions?
  2. Added. Users only need to specify the method=PURGE to delete cache file. Please review it.

Thanks.

@agile6v
Copy link
Member Author

agile6v commented Mar 3, 2020

And I have other new questions:

  1. need some e2e test case
  2. need a way to purge the old cache
  1. e2e test cases I haven't figured it out yet. Do you have any suggestions?
  2. Added. Users only need to specify the method=PURGE to delete cache file. Please review it.

Thanks.

@moonming @membphis

lua/apisix/plugins/proxy-cache.lua Outdated Show resolved Hide resolved
@agile6v
Copy link
Member Author

agile6v commented Mar 5, 2020

Do you have any suggestions for the test cases?

Moreover, I think I should figure out the logic of the APISIX test cases.

Thanks.

@agile6v
Copy link
Member Author

agile6v commented Mar 12, 2020

Please review @moonming @membphis

@moonming
Copy link
Member

LGTM. Need add this plugin to README.md.
@membphis please take a look about how to add test cases.

@agile6v
Copy link
Member Author

agile6v commented Mar 12, 2020

OK. I'll add it to README. Basically test cases are covered.

@agile6v
Copy link
Member Author

agile6v commented Mar 14, 2020

@membphis ping

t/plugin/proxy-cache.t Outdated Show resolved Hide resolved
t/plugin/proxy-cache.t Show resolved Hide resolved
lua/apisix/plugins/proxy-cache.lua Show resolved Hide resolved
@membphis
Copy link
Member

@agile6v It seems that you have to rebase your branch, this branch has conflicted with master branch.

@agile6v
Copy link
Member Author

agile6v commented Mar 15, 2020

@agile6v It seems that you have to rebase your branch, this branch has conflicted with master branch.

Resolved.

@membphis
Copy link
Member

LGTM, we can merge it after run the test cases.

@membphis membphis merged commit 864aa16 into apache:master Mar 15, 2020
@membphis
Copy link
Member

merged, many thx @agile6v

@agile6v agile6v deleted the dev branch March 16, 2020 12:03
SaberMaster pushed a commit to SaberMaster/incubator-apisix that referenced this pull request Jun 30, 2020
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.

Feature: introduce proxy caching plugin
3 participants