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

goss serve cache should separate results from output #612

Closed
petemounce opened this issue Sep 10, 2020 · 1 comment · Fixed by #814
Closed

goss serve cache should separate results from output #612

petemounce opened this issue Sep 10, 2020 · 1 comment · Fixed by #814

Comments

@petemounce
Copy link
Collaborator

Describe the feature:
Implementation detail change.

Sliced work from #609 (comment) discussion.

goss serve has a cache. Currently, this caches rendered output.

#609 introduces the possibility of different rendered output for the same validate-run.

It's important that the rendered outputs all tie back to the same instance of a validate-run.

Describe the solution you'd like

Cache the validate-run results only, and don't cache the output rendering

  • ... because the hypothesis is that the output rendering is trivially fast

Describe alternatives you've considered

  1. It would be more time-efficient (but cost memory) to cache in two stages

    1. the result of a validate run
    2. the rendered output

    ... but

    • it would introduce complexity to the cache
      • need to expire the output cache entries that are tied to a results-cache entry, when that expiry occurs
      • ideally want configurability options like max-cache-size
    • we don't (currently) have a convenient way to repeatably profile this to see whether
      • this would in fact be a net-positive in speed, compared to a regression
      • the extra memory is worthwhile
      • the speed is worth the memory + complexity
@aelsabbahy
Copy link
Member

aelsabbahy commented Sep 11, 2020

because the hypothesis is that the output rendering is trivially fast

That would be my vote too. Once cache on the results should be enough for now.. if later on there is a need for caching results that can be a further enhancement.

Approved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants