[ https://issues.apache.org/jira/browse/HBASE-25913?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17351285#comment-17351285 ]
Andrew Kyle Purtell edited comment on HBASE-25913 at 5/25/21, 6:38 PM: ----------------------------------------------------------------------- {quote}Since HBase doesn't provide any RS-level atomicity guarantees that I'm aware of, should this be done at the Region level rather than RS to allow more throughput? {quote} EnvironmentEdge provides a convenient an existing place to track the 'last observed time'. It is already in use, just not universally as it is supposed to be (which we can fix). That is the thought. It is a process singleton, though. Although, we will only use {{EnvironmentEdge#currentTimeAdvancing}} in a few places for a very specific reason. This could be augmented to track the last observed time in a table keyed by something like region name. Imagine an API like long {{EnvironmentEdge#currentTimeAdvancing(String clockName)}} . Not sure if this will work out, just spitballing here. We can try it. There is now more overhead, but is it significantly more. Perhaps not. EnvironmentEdge#addClock(String name) – called when region is opened EnvironmentEdge#removeClock(String name) – called when region is closed EnvironmentEdge#currentTime() – just the System.currentTimeMillis we have now EnvironmentEdge#currentTime(String name) – track the last seen time with 'name' EnvironmentEdge#currentTimeAdvancing)(String name) – don't return until the current clock value is more recent than the last seen time tracked with 'name' was (Author: apurtell): {quote}Since HBase doesn't provide any RS-level atomicity guarantees that I'm aware of, should this be done at the Region level rather than RS to allow more throughput? {quote} EnvironmentEdge provides a convenient an existing place to track the 'last observed time'. It is already in use, just not universally as it is supposed to be (which we can fix). That is the thought. It is a process singleton, though. Although, we will only use {{EnvironmentEdge#currentTimeAdvancing}} in a few places for a very specific reason. This could be augmented to track the last observed time in a table keyed by something like region name. Imagine an API like long {{EnvironmentEdge#currentTimeAdvancing(String clockName)}} . Not sure if this will work out, just spitballing here. We can try it. There is now more overhead, but is it significantly more. Perhaps not. > Introduce EnvironmentEdge.currentTimeAdvancing > ---------------------------------------------- > > Key: HBASE-25913 > URL: https://issues.apache.org/jira/browse/HBASE-25913 > Project: HBase > Issue Type: Sub-task > Reporter: Andrew Kyle Purtell > Assignee: Andrew Kyle Purtell > Priority: Major > Fix For: 3.0.0-alpha-1, 2.5.0 > > > Introduce new {{EnvironmentEdge#currentTimeAdvancing}} which ensures that > when the current time is returned, it is the current time in a different > clock tick from the last time the {{EnvironmentEdge}} was used to get the > current time. > When processing mutations we substitute the {{Long.MAX_VALUE}} timestamp > placeholder with a real placeholder just before committing the mutation. The > current code gets the current time for timestamp substitution while under row > lock and mvcc. We will simply use {{EnvironmentEdge#currentTimeAdvancing}} > instead of {{EnvironmentEdge#currentTime}} at this point in the code to > ensure we have seen the clock tick over. When processing a batch of mutations > (doMiniBatchMutation etc) we will call {{currentTimeAdvancing}} only once. > This means the client cannot bundle cells with wildcard timestamps into a > batch where those cells must be committed with different timestamps. Clients > must simply not submit mutations that must be committed with guaranteed > distinct timestamps in the same batch. Easy to understand, easy to document, > and it aligns with our design philosophy of the client knows best. > It is not required to handle batches as proposed. We could guarantee a > distinct timestamp for every mutation in a batch. Count the number of > mutations, call this M. Acquire all row locks and get the current time. Then, > wait for at least M milliseconds. Then, set the first mutation timestamp with > this value and increment by 1 for all remaining. Then, do the rest of > mutation processing as normal. I don't think this extra waiting to reserve > the range of timestamps is necessary. See reasoning in above paragraph. > Mentioned here for sake of discussion. > It will be fine to continue to use {{EnvironmentEdge#currentTime}} everywhere > else. In this way we will only potentially spin wait where it matters, and > won't suffer serious overheads during batch processing. -- This message was sent by Atlassian Jira (v8.3.4#803005)