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

cli list blocks usability improvements #403

Merged
merged 6 commits into from
Dec 10, 2020

Conversation

mdisibio
Copy link
Contributor

@mdisibio mdisibio commented Dec 9, 2020

What this PR does:
A few minor usability improvements to the list blocks command:

  • Load block meta in parallel, up to 10 at a time, to run much faster.
  • Only show active blocks unless --include-compacted flag is specified (and show new flag column in results)
  • Sort by block end time instead of window and compaction level. This previous ordering was somewhat awkward because it put the newest blocks somewhere in the middle.
  • Add Age column which is time since block end time

Which issue(s) this PR fixes:
n/a

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Copy link
Contributor

@achatterjee-grafana achatterjee-grafana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added copy-edit suggestions.

docs/tempo/website/cli/_index.md Outdated Show resolved Hide resolved
docs/tempo/website/cli/_index.md Outdated Show resolved Hide resolved
docs/tempo/website/cli/_index.md Outdated Show resolved Hide resolved
docs/tempo/website/cli/_index.md Outdated Show resolved Hide resolved
Copy link
Contributor

@annanay25 annanay25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I like the simplistic implementation of the boundedWaitGroup. :)

Let's also add to the changelog as its a user facing change.

@mdisibio
Copy link
Contributor Author

Nice! I like the simplistic implementation of the boundedWaitGroup. :)

Credit goes to a different author. Was looking around for popular solutions to the limited concurrency and came across it via: /r/golang comment and playground

mdisibio and others added 3 commits December 10, 2020 08:51
Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>
Copy link
Contributor

@achatterjee-grafana achatterjee-grafana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doc and code comments look good. Just a couple more copy -edit request.

CHANGELOG.md Show resolved Hide resolved
cmd/tempo-cli/cmd-list-blocks.go Show resolved Hide resolved
@joe-elliott joe-elliott merged commit 37e596e into grafana:master Dec 10, 2020
@mdisibio mdisibio deleted the cli-compacted branch February 3, 2021 18:09
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.

4 participants