sodonnel commented on PR #7402:
URL: https://github.com/apache/ozone/pull/7402#issuecomment-2482965692

   Thanks for reducing the changed files. Its down to 47 now from 97, which is 
a big improvement.
   
   I still think its confusing that we have two structures (missingContainers 
map and ContainerIDsTable) to track what is going on.
   
   If we want to block creating a container that has been created before, can 
we just check for existence in the ContainerIDsTable? Right now we check the 
missingTable and then add an entry to the containerIDTable.
   
   If we remove a container, there are two boolean parameters - markMissing and 
removeFromDB - is it ever valid to pass "true, true" so it marks it missing and 
removes it from the DB?
   
   Provided we always add to the containerIDsTable on new containerCreation, 
then we should be able to check for previous creation from it too.
   
   It feels like we can remove the container for a failed volume we don't touch 
the containerIDs table. Something that tries to recreate it will get blocked by 
the existence chance against the table.
   
   Or, we remove it "safely" via replication, balancer and we remove it from 
the DB.
   
   If we just use the containerID table, then you don't need to iterate it at 
startup and checkoff all the containers. You also don't need to hold anything 
in memory as the RocksDB will cache things itself. However we may need to 
populate the table from all existing containers on disk.
   
   I also suspect the code would be more understandable if we didn't overload 
add/removeContainer in ContainerSet with the booleans, and instead create a new 
method `add???` and `remove???` which are named more inline with what they are 
doing, eg remove, removeSafely, removeUnexpectedly, or something along those 
lines.


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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to