Ming Ma It's a LinkedHashSet to cover the cases you state.
The motivation of randomization is to make sure we don't end up scanning the same blocks if there isn't much update to postponedMisreplicatedBlocks. <...> What is the reason to change from HashSet to LinkedHashSet, to have some order guarantee? HashSet is faster than LinkedHashSet. <...> During the scan, if a block is marked as POSTPONE, it will be removed from postponedMisreplicatedBlocks first and add it back later via rescannedMisreplicatedBlocks. Can it just remove those blocks from postponedMisreplicatedBlocks that aren't marked as POSTPONE without using rescannedMisreplicatedBlocks?
The LHS iterator is effectively functioning as a cheap circular array with acceptable insert/remove performance. Yes, the order guarantee ensures all blocks are visited w/o "skipping" to a random location. The block always has to be removed and re-inserted for the ordering to work. Otherwise the same blocks will be scanned over and over again.
Does the latency of couple seconds come from HashSet iteration? Some quick test indicates iterating through 5M entries of a HashSet take around 50ms.
Yes, the cycles wasted to skip the through the set were the killer. A micro-benchmark doesn't capture the same chaotic runtime environment of over a 100 threads competing for resources. Kihwal says (ironically) the performance "improved" under 5M.
In multiple production incidents, the postponed queue backed up with 10-20M+ blocks. Scans determined the blocks are all still postponed. Pre-patch, the cycles took up to a few seconds and averaged in the many hundreds of ms. Performance was obviously atrocious. Post-patch, same scans took no more than a few ms.