- 
                Notifications
    
You must be signed in to change notification settings  - Fork 25.6k
 
Optimize time-series source operator #127095
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
Conversation
cce64e6    to
    d4f7e9b      
    Compare
  
    d4f7e9b    to
    57ab327      
    Compare
  
    | 
           Pinging @elastic/es-analytical-engine (Team:Analytics)  | 
    
| 
           Pinging @elastic/es-storage-engine (Team:StorageEngine)  | 
    
        
          
                ...te/src/main/java/org/elasticsearch/compute/lucene/TimeSeriesSortedSourceOperatorFactory.java
          
            Show resolved
            Hide resolved
        
              
          
                ...te/src/main/java/org/elasticsearch/compute/lucene/TimeSeriesSortedSourceOperatorFactory.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...te/src/main/java/org/elasticsearch/compute/lucene/TimeSeriesSortedSourceOperatorFactory.java
          
            Show resolved
            Hide resolved
        
              
          
                ...te/src/main/java/org/elasticsearch/compute/lucene/TimeSeriesSortedSourceOperatorFactory.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...te/src/main/java/org/elasticsearch/compute/lucene/TimeSeriesSortedSourceOperatorFactory.java
          
            Show resolved
            Hide resolved
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, a few nits and questions about further improvements. Let's also have Martijn double-check the lucene part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, great job Nhat 👍
        
          
                ...te/src/main/java/org/elasticsearch/compute/lucene/TimeSeriesSortedSourceOperatorFactory.java
          
            Show resolved
            Hide resolved
        
              
          
                x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/DocVector.java
          
            Show resolved
            Hide resolved
        
              
          
                ...te/src/main/java/org/elasticsearch/compute/lucene/TimeSeriesSortedSourceOperatorFactory.java
          
            Show resolved
            Hide resolved
        
              
          
                ...te/src/main/java/org/elasticsearch/compute/lucene/TimeSeriesSortedSourceOperatorFactory.java
          
            Show resolved
            Hide resolved
        
      | 
           @kkrik-es @martijnvg Thanks for reviewing.  | 
    
This query against the TSDB track took 50 seconds and was reduced to 19 seconds with this changes.
This change introduces several optimizations to improve the performance of the time-series source operator:
_tsidand another for@timestamp. This avoids repeatedly comparing large_tsidvalues while iterating over a single_tsid._tsidmultiple times.