[GitHub] [ozone] bshashikant commented on pull request #1980: HDDS-4651: Distributed Sequence ID Gen.

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

bshashikant commented on pull request #1980:
URL: https://github.com/apache/ozone/pull/1980#issuecomment-789701900


   > > Do we need to handle overflow for "long" used here for lastId?
   > 
   > Thanks for the advice, I've add the check.
   > 
   > > With these approach, we won't have monotonically increasing containerIds or block ids. IDs will be monotonically increasing within a batch.
   > 
   > Could you find a situation that containerId or blockId is not monotonically increasing ? For non-HA mode, it is straightword that the id is monotonically increasing. For HA mode, since only leader can request a nextID, and leader will always refresh lastId from db and allocate a new batch, a new leader will always generate larger id than previous leader, thus monotonically increasing too.
   
   What i meant was, containerIds are not sequentially increasing which is a departure from the current behaviour which is strictly sequentially increasing containerIds. and we are using the same seqID generator for both Container as well as block.
   This may become the reason for scale limitation.
   
   Instead of that, why can't we use lastAppliedIndex as base for containerIds.
   > 
   > > We need to handle the upgrade scenario. This needs to be handled before the branch gets merged otherwise all existing setups won't work.
   > 
   > We have a discussion in the design doc:
   > 
   > > need a hook to be used in upgrade, to ensure the monotonicity of id during switch from previous ids to current solution:
   > > localId : the initial value should be UniqueId() + 1
   > > containerID: scan the container db, figure the largest containerID ever used.
   > > delTx: read from DeletedBlocksTXTable, see HDDS-4477 contributed by Lokesh.
   > 
   > So, we can simply use `UniqueId() + 1` as `INVALID_SEQUENCE_ID`, since containerID and delTx are starting from 1, practically they can not be larger than `UniqueId() + 1`.
   
   Are u going to address these changes in subsequent PR as this will be required for branch merge?
   
   


----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org