Skip to content

Add log for delete table API#3763

Merged
jackjlli merged 3 commits intomasterfrom
add-log-for-delete-table-api
Feb 1, 2019
Merged

Add log for delete table API#3763
jackjlli merged 3 commits intomasterfrom
add-log-for-delete-table-api

Conversation

@jackjlli
Copy link
Member

This PR adds logs for deleteTable API. We've seen this API taking too much time to proceed. Adding logs helps to track which part is the bottle neck.

@codecov-io
Copy link

codecov-io commented Jan 30, 2019

Codecov Report

Merging #3763 into master will increase coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3763      +/-   ##
============================================
+ Coverage     67.09%   67.17%   +0.08%     
  Complexity        4        4              
============================================
  Files          1027     1027              
  Lines         50799    50814      +15     
  Branches       7091     7091              
============================================
+ Hits          34082    34135      +53     
+ Misses        14400    14350      -50     
- Partials       2317     2329      +12
Impacted Files Coverage Δ Complexity Δ
...ntroller/helix/core/PinotHelixResourceManager.java 60.79% <100%> (+0.63%) 0 <0> (ø) ⬇️
...a/manager/realtime/RealtimeSegmentDataManager.java 50% <0%> (-50%) 0% <0%> (ø)
...lix/EmptyBrokerOnlineOfflineStateModelFactory.java 86.66% <0%> (-13.34%) 0% <0%> (ø)
...pinot/core/operator/docidsets/OrBlockDocIdSet.java 84.9% <0%> (-13.21%) 0% <0%> (ø)
.../impl/dictionary/LongOffHeapMutableDictionary.java 81.81% <0%> (-10.91%) 0% <0%> (ø)
.../BrokerResourceOnlineOfflineStateModelFactory.java 49.15% <0%> (-10.17%) 0% <0%> (ø)
...impl/dictionary/DoubleOnHeapMutableDictionary.java 68.88% <0%> (-6.67%) 0% <0%> (ø)
.../impl/dictionary/FloatOnHeapMutableDictionary.java 86.66% <0%> (-6.67%) 0% <0%> (ø)
...e/operator/dociditerators/SortedDocIdIterator.java 61.11% <0%> (-2.78%) 0% <0%> (ø)
...va/org/apache/pinot/common/data/TimeFieldSpec.java 91.35% <0%> (-1.24%) 0% <0%> (ø)
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94b34e2...f538819. Read the comment docs.

Copy link
Contributor

@mcvsubbu mcvsubbu left a comment

Choose a reason for hiding this comment

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

Please make sure that the table name (with type) is there in all the tables. Time is not necessary, since the log message should indicate the time of log.
I would format the message something like this:
"Deleting table {}: Start"
"Deleting table {} : Removed from broker idealstate"
"Deleting table {} : SOMETHNG ELSE"
etc.
"Deleting table {}: Done"
so that we can just look for the table that we want, and get all the deletion logs.

@jackjlli
Copy link
Member Author

@mcvsubbu Comment addressed

Copy link
Contributor

@mcvsubbu mcvsubbu left a comment

Choose a reason for hiding this comment

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

why do we need the time in each log/? The log line provides a msec time.

@jackjlli jackjlli force-pushed the add-log-for-delete-table-api branch from 71b6015 to aac9e03 Compare January 31, 2019 01:05
@jackjlli jackjlli force-pushed the add-log-for-delete-table-api branch from aac9e03 to f54d597 Compare January 31, 2019 01:13
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.

3 participants