[GitHub] [phoenix] gjacoby126 edited a comment on pull request #823: PHOENIX-5991 IndexRegionObserver should not overwrite mutation timest…

[GitBox <gi...@apache.org>]

gjacoby126 edited a comment on pull request #823:
URL: https://github.com/apache/phoenix/pull/823#issuecomment-654998520


   @kadirozde - since HBase timestamps are ultimately derived from physical timestamps, rather than, say HLCs, we're already prone to weird effects from clock skew (such as different rs's having different clocks). This improves that in one aspect, since when the client-set timestamp "wins" it'll be consistent across region servers.
   
   I'm curious about the implications of letting the server timestamp win when the server is behind the client. What if the client clock is slightly ahead of a server? Don't we go back to inconsistent timestamps then?
   
   It seems like a preference for either the client or the server can cause skew-related problems.
   
   When we honor the client timestamps, we get consistent timestamps across a logical operation no matter how many region servers it involves. However, we can get weird behavior where two clients with clock skew wrt each other can invert the order of operations.
   
   When we honor the server timestamps, we can enforce an order between client operations regardless of the clients' clocks, but we can get inconsistent timestamps within each operation. 
   
   I can think of use cases I've worked with in the past that have a strong need for either consistent timestamps across a long-running operations and others with lots of small operations where order of operations across clients _really_ matters , so I'm thinking this needs to be configurable. 
   
   @gjacoby126 - Thank you for the feedback. I agree with on almost all of the points you made. I am not sure about having a configuration parameter about this due to the conflict with SCN queries. I think we need to honor the client timestamps and it will improve the current situation as you pointed out. Initially, I started with the following logic:
   long now = getTimestamp(firstMutation);
   if (now ==  HConstants.LATEST_TIMESTAMP) {
        now = EnvironmentEdgeManager.currentTimeMillis()
   }
   
   Then I noticed that the client always sets the timestamp for mutations in my ITs. I changed the logic. I think as you said, we should either use the client or server timestamp. I will revert the logic, and so we will use the client timestamp when the client set it. What do you think?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org