Content-Length: 232623 | pFad | http://github.com/vitessio/vitess/issues/17925

18 Bug Report: Do not use timestamps for schema_migrations and vreplication_log · Issue #17925 · vitessio/vitess · GitHub
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.

Sign up for GitHub

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

Jump to bottom

Bug Report: Do not use timestamps for schema_migrations and vreplication_log #17925

Open
corbantek opened this issue Mar 6, 2025 · 2 comments · May be fixed by #17910
Open

Bug Report: Do not use timestamps for schema_migrations and vreplication_log #17925

corbantek opened this issue Mar 6, 2025 · 2 comments · May be fixed by #17910
Labels
Component: General Changes throughout the code base Type: Bug

Comments

@corbantek
Copy link
Contributor

Overview of the Issue

At HubSpot we have a feature that allows us to sync data across different deployments of Vitess and is implemented by using MySQL binlogs.

When this was used in conjunction with Vitess migrations we were hitting the default timestamp bug whenever we run a new vitess migration, even though nothing we had created had default timestamps. We looked through the binlogs and sure enough, the vreplication tables have default timestamps...

After much debugging, we found that this happens whenever a CREATE TABLE/ALTER TABLE is run with a TIMESTAMP field (no matter how this was configured). The solution here is to create the column as a DATETIME field, which is virtually the same as TIMESTAMP for this use case.

Now when a vitess migration is started/stopped/completed, the CREATE TABLE/ALTER table statements are still run, but they're using datetime instead of timestamp, so the explicit_defaults_for_timestamp line does not make it into the binlogs.

Reproduction Steps

N/A

Binary Version

vitessio/main

Operating System and Environment details

MYSQL_VERSION=8.0.33

`uname -sr` 

Linux 6.1.109-hs83.el9.x86_64

Log Fragments


      		
    

      
  





        



            

              
            

        

      


      
    








      

    



      

  
            

    

      
    

    


      

          @corbantek
corbantek




            added
      

  Needs Triage

  This issue needs to be correctly labelled and triaged

      

  Type: Bug


  labels


      Mar 6, 2025

    

  



  

  

    
  


  

      

    @corbantek
corbantek



    linked a pull request
    
      Mar 6, 2025
    

    that will
    close
    this issue





  


  

      
   Draft

  





  
  

      
        
      
      

  
    
    
  


    5 tasks
  





  









      

  
      



  

  @shlomi-noach





  


    

      

  

    

        
    
    

  


      
          
  
      
            Copy link

  

      
    

  


  

      

  
    Contributor


      

  


  

    

  





      


        


  
    

      
          
Hi! Putting some bullet numbers here so we can discuss each point separately.


    

  1. 

    If I'm reading this correctly, the proposal here is that Vitess should never use the TIMESTAMP data type? (thereby also in vdiff, vdiff_log, vdiff_table?)
    

    2. 

  2. 

    From the bug report it looks like the problem is limited to using replication filters, is that correct?
    

    3. 

  3. 

    Generally speaking, I don't see DATETIME as a good replacement to TIMESTAMP because of its lack of timezone awareness. Again generally speaking, I think in most cases TIMESTAMP is the more correct type to use.
    

    4. 

  4. 

    I appreciate there's a MySQL bug related to timestamps. However, this also seems to be not only not in vitess, but also does not affect vitess. It only affects an external tool that uses Vitess. Is this correct?
    

    5. 


      		
    

  





        


            

            

              
            

        

      


      

            
  

    
  
    
    

  

  



    












      

  
            

    

      
    

    


      

          @shlomi-noach
shlomi-noach




            added
      

  Component: General

  Changes throughout the code base

    and removed
      

  Needs Triage

  This issue needs to be correctly labelled and triaged

  labels


      Mar 10, 2025

    

  








      

  
      



  

  @corbantek





  


    

      

  

    

        
    
    

  


      
          
  
      
            Copy link

  

      
    

  


  

      

  
    Contributor


      

  Author


  


  

    

      

      
            corbantek
  

      

      

      commented


        Mar 12, 2025


      
  •

  

    
      

        
          edited
          
        
        
      

    
    
      
        
      Loading

      
    
  



    


  





      


        


  
    

      
          


Hi! Putting some bullet numbers here so we can discuss each point separately.


    

  1. I appreciate there's a MySQL bug related to timestamps. However, this also seems to be not only not in vitess, but also does not affect vitess. It only affects an external tool that uses Vitess. Is this correct?
    2. 





Yes this is an external feature (that exists in or Vitess fork) that allows us to replicate tables to other instances of Vitess using binglogs.




    

  1. If I'm reading this correctly, the proposal here is that Vitess should never use the TIMESTAMP data type? (thereby also in vdiff, vdiff_log, vdiff_table?)
    2. 





Correct to be more complete we could update these tables, however our Cross-Vitess Replication doesn't support multi-sharded keyspaces at the moment, so these tables were not updated.




    

  1. From the bug report it looks like the problem is limited to using replication filters, is that correct?
    2. 





Correct, due to the mysqlbinlog bug, the line for explicit_defaults_for_timestamp still gets into the binlogs, which causes all the timestamp fields to get unwanted defaults




    

  1. Generally speaking, I don't see DATETIME as a good replacement to TIMESTAMP because of its lack of timezone awareness. Again generally speaking, I think in most cases TIMESTAMP is the more correct type to use.
    2. 





For our use, DATETIME is analogous with TIMESTAMP as we set everything to UTC, but I understand if someone is using timezones in a different way it would be an issue.  The upside is DATETIME goes beyond 2038 :)


Our goal is upstream all of our fixes at HubSpot, but if this minor change causes more choas that good, I'm okay with keeping this forked. Maybe a compromise is have some sort of dual-mode  initializer that defaults to timestamp but can also use "datetime" if a flag is supplied...

      		
    

  





        


            

            

              
            

        

      


      

            
  

    
  
    
    

  

  



    
















  
  

    

      

  





  




  
  

              

  
    Sign up for free
    to join this conversation on GitHub.
    Already have an account?
    Sign in to comment


  



  






  
                  




    

  


      
  

    Assignees
  



        

    No one assigned







      


  


  

    Labels
  



    

      

    Component: General

  Changes throughout the code base
  

    Type: Bug









      

  

    

        

    Projects
  


        




    None yet




  



      


  

    
  

    Milestone
  


      No milestone





    
      
          


  

    

      
        

          
    

    Development
  




            
  
Successfully merging a pull request may close this issue.





    


  
          
      Do not use timestamps for schema_migrations and vreplication_log

    
      HubSpot/vitess
    





      
    

  




      

    

  
      

  

    

      2 participants
    

    

        
          @shlomi-noach 
        
          @corbantek 
    

  






    

    




    






  

          

        



    



  





    

  


  


          



    



  

    

    

    





    


    

    

  
    






    
    

    
    

    
    

    
    

    
    

    
    

    
    

    
    


        
        
        
        
         ApplySandwichStrip

    


pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!

--- a PPN by Garber Painting Akron.  With Image Size Reduction included!
Fetched URL: http://github.com/vitessio/vitess/issues/17925
Alternative Proxies:
Alternative Proxy
pFad Proxy
pFad v3 Proxy
pFad v4 Proxy