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

feat: add profiler to query command in cli #21006

Merged
merged 9 commits into from
Mar 23, 2021
Merged

Conversation

russorat
Copy link
Contributor

@russorat russorat commented Mar 19, 2021

Closes #21005

influx query -p operator "buckets() |> limit(n:1)"
influx query -p query,operator "buckets() |> limit(n:1)"
influx query -p query "buckets() |> limit(n:1)"

  • CHANGELOG.md updated with a link to the PR (not the Issue)
  • Well-formatted commit messages
  • Rebased/mergeable
  • Tests pass
  • http/swagger.yml updated (if modified Go structs or API)
  • Feature flagged (if modified API)
  • Documentation updated or issue created (provide link to issue/pr)
  • Signed CLA (if not already signed)

Copy link
Contributor

@danxmoran danxmoran left a comment

Choose a reason for hiding this comment

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

Other than my one question:

  • Can you update CHANGELOG.md?
  • How does passing -p change the output of the CLI? Can we update cmd/influx/query_test.go to check the new behavior?

}

if queryFlags.profiler {
body_map["extern"] = map[string]interface{}{
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move the map[string]interface{}{...} definition to a top-level const? IMO it'd make it more clear that it's a "magic" constant instead of something that depends on user input.

Copy link
Contributor

Choose a reason for hiding this comment

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

Once it's moved, is there a docs page we could link to in a comment that explains the structure? In case something changes in the future and we need to figure out how this should be updated to match

@danxmoran danxmoran self-assigned this Mar 22, 2021
@russorat
Copy link
Contributor Author

example output

➜  influxdb git:(add-profiler-to-cli) ✗ ./bin/darwin/influx query -p "buckets() |> limit(n:1)"
Result: _result
Table: keys: [organizationID]
 organizationID:string             name:string               id:string  retentionPolicy:string         retentionPeriod:int  
----------------------  ----------------------  ----------------------  ----------------------  --------------------------  
      844910ece80be8bc                  1_demo        8944be6ebc10e63d                                                   0  
Result: _profiler
Table: keys: [_measurement]
   _measurement:string           TotalDuration:int         CompileDuration:int           QueueDuration:int            PlanDuration:int         RequeueDuration:int         ExecuteDuration:int             Concurrency:int            MaxAllocated:int          TotalAllocated:int    RuntimeErrors:string                                                                                         flux/query-plan:string  
----------------------  --------------------------  --------------------------  --------------------------  --------------------------  --------------------------  --------------------------  --------------------------  --------------------------  --------------------------  ----------------------  -------------------------------------------------------------------------------------------------------------  
        profiler/query                   219390091                      211442                       14667                           0                           0                   219124418                           0                       16000                           0                          digraph {
  localBuckets
  limit1
  generated_yield
  localBuckets -> limit1
  limit1 -> generated_yield
}
Table: keys: [_measurement]
   _measurement:string                    Type:string            Label:string                   Count:int             MinDuration:int             MaxDuration:int             DurationSum:int            MeanDuration:float  
----------------------  -----------------------------  ----------------------  --------------------------  --------------------------  --------------------------  --------------------------  ----------------------------  
     profiler/operator  *universe.limitTransformation                  limit1                           1                       27012                       27012                       27012                         27012  
     profiler/operator         *execute.sourceDecoder            localBuckets                           1                   214453868                   214453868                   214453868                     214453868 

danxmoran
danxmoran previously approved these changes Mar 22, 2021
@danxmoran danxmoran dismissed their stale review March 22, 2021 20:25

Talked on Slack - would be better to support enabling specific profilers

@russorat russorat requested a review from danxmoran March 22, 2021 21:52
@danxmoran danxmoran merged commit 1b7d5f3 into master Mar 23, 2021
@danxmoran danxmoran deleted the add-profiler-to-cli branch March 23, 2021 19:47
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.

InfluxDB CLI: Add a flag for profiling a query to the influx query command
2 participants