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

[BUG] Whispers under symlink are not returned for queries with wildcards #572

Open
interfan7 opened this issue Nov 8, 2023 · 6 comments
Labels

Comments

@interfan7
Copy link

Describe the bug
(also described in carbonapi issues)
For metrics in Whisper files which are under symlink (in the path):

  • Querying the metric specifically, e.g. x.y4.z.w, is returned as expected, even if its Whisper file path contains a symlink.
  • Querying multiple metrics based on text-search, i.e. x.y*.z.w, returns only the metrics whose backing Whisper files are not under symlink.

Seems like something in the search engine is effected by a path component being a symlink, which in our case is directory y.

Logs
No error messages in both go-carbon and carbonapi while reproducing it.
In go-carbon log I do see INFO messages of metrics which are not under symlink.
In carbonapi log I see INFO messages with the query search text i.e. x.y*.z.w.

Go-carbon Configuration:
Mostly default, I will provide config if a contributor requests. I believe it may not be relevant though given the nature of the issue.

Metric retention and aggregation schemas
See above.

Simplified query (if applicable)
See description 👆🏻

Additional context
go-carbon 0.17.1 and carbonapi 0.16.0~1.

@interfan7 interfan7 added the bug label Nov 8, 2023
@deniszh
Copy link
Member

deniszh commented Nov 10, 2023

Hi @interfan7,

Could you please share your go-carbon.conf file and please provide details where exactly symlink located and where it's pointing to and what's files are located there (because it's not clear from description above).
For example, whisper files located in /var/lib/whisper, symlink is /var/lib/whisper/path, pointing to /var/lib/anotherwhisper, where's only *.wsp files located.

@deniszh
Copy link
Member

deniszh commented Nov 10, 2023

Well, I just checked code - carbonserver uses filepath.Walk which is do not support symlink by design. In theory, it can be implemented, but can significantly affect code complexity and performance, I'm doubt that we will implement it in near future.

@interfan7
Copy link
Author

@deniszh

Would you mind to note what is the code complexity? Is it just the addition of this logic (kind of pseudo-code because I'm not a Go programmer) inside the walk:

if (file is symlink)
{
   (realpath, error) = symlink_root = filepath.EvalSymlinks(file)
   if (!error)
   {
      filepath.WalkDir(realpath, same_fn)
   }
}

This whole logic could be wrapped under user-defined config flag for performance preference although I'd hope the added logic is negligible in performance versus the actual read of a Whisper file.

BTW according to the doc filepath.WalkDir is more efficient than filepath.Walk.

@deniszh
Copy link
Member

deniszh commented Nov 11, 2023

Hi @interfan7

I'm not considering myself as professional Go programmer, but code above is more complex than just simple filepath.Walk, isn't it? :)
I just want to emphasize that we'll not use symlink in prod, hence we will probably not implement symlink support in go-carbon in the near future. Also, please note that we're talking about carbonserver here and all related machinery (trie/trigram indices etc), probably normal carbonlink still support symlinks. Not sure if it will help in your case, though.
PRs from community are always welcome and appreciated, of course.

@deniszh
Copy link
Member

deniszh commented Nov 11, 2023

BTW according to the doc filepath.WalkDir is more efficient than filepath.Walk.

Yes, but WalkDir loads dir content in memory, can be quite heavy for big directories.

@Civil
Copy link
Member

Civil commented Nov 11, 2023

My .02: if you are traversing symlinks you need to some way to workaround endless loops as well.

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

No branches or pull requests

3 participants