I got this patch in conjunction with
HDFS-1108 and HDFS-1971 to properly replicate the creation of a new file, but then moved on to working on setReplication and ran into issues there. The issue I'm seeing is this:
1) Active NN receives setReplication to drop some file's replication from 3 to 1
2) It writes OP_SET_REPLICATION to its log, invalidates two replicas, and returns
3) The DNs report BLOCK_INVALIDATED back to both the ActiveNN and SBNN.
4) The SBNN hasn't received the OP_SET_REPLICATION yet, so it marks the block as under-replicated.
In the case of raising replication (eg from 1 to 3) we get the opposite problem: the SBNN marks the block as over-replicated and adds two of the replicas to its invalidation list.
Generation stamps don't help here, because changing replication level of a block doesn't change its gen-stamp (and it shouldn't).
The solution I'm thinking of is that we have to track the transaction ID when we send comments to DNs. So, if a setReplication command at txid=123 causes invalidation of two blocks, we'd send the INVALIDATE command with "txid=123". Then, when the DN does delete these blocks, it would ack back with that txid to both NNs. The SBNN wouldn't process this message until it had loaded that txid.
A bit of a simplification from this would be that any command being processed from an NN will include the NN's txid, which the DN records in BPOfferService as "latestCommandTxId". Then, any calls to the NN would include this txid. This is a bit more conservative than tracking it with each block command, but probably less prone to bugs.
I plan to take a pass at implementing this latter approach.