[SPARK-19520][streaming] Do not encrypt data written to the WAL. #16862
Conversation
Spark's I/O encryption uses an ephemeral key for each driver instance. So driver B cannot decrypt data written by driver A since it doesn't have the correct key. The write ahead log is used for recovery, thus needs to be readable by a different driver. So it cannot be encrypted by Spark's I/O encryption code. The BlockManager APIs used by the WAL code to write the data automatically encrypt data, so changes are needed so that callers can to opt out of encryption. Aside from that, the "putBytes" API in the BlockManager does not do encryption, so a separate situation arised where the WAL would write unencrypted data to the BM and, when those blocks were read, decryption would fail. So the WAL code needs to ask the BM to encrypt that data when encryption is enabled; this code is not optimal since it results in a (temporary) second copy of the data block in memory, but should be OK for now until a more performant solution is added. The non-encryption case should not be affected. Tested with new unit tests, and by running streaming apps that do recovery using the WAL data with I/O encryption turned on.
|
Test build #72614 has finished for PR 16862 at commit
|
|
Kafka test error (hopefully unrelated?). Retest this please |
|
Test build #72654 has finished for PR 16862 at commit
|
| level: StorageLevel, | ||
| tellMaster: Boolean = true): Boolean = { | ||
| tellMaster: Boolean = true, | ||
| encrypt: Boolean = false): Boolean = { |
There was a problem hiding this comment.
I think its worth documenting this param. At first I was going to suggest that it should be called allowEncryption like the other one, but I realize its more complicated than that. Maybe something like
If true, the given bytes should be encrypted before they are stored. Note that in most cases, the given bytes will already be encrypted if encryption is on. An important exception to this is with the streaming WAL. Since the WAL does not support encryption, those bytes are generated un-encrypted. But we still encrypt those bytes before storing in the block manager.
Maybe too wordy but I think its worth documenting.
squito
left a comment
squito
left a comment
There was a problem hiding this comment.
one minor suggestion for a doc, otherwise lgtm
| partition.walRecordHandle) | ||
| if (storeInBlockManager) { | ||
| blockManager.putBytes(blockId, new ChunkedByteBuffer(dataRead.duplicate()), storageLevel) | ||
| blockManager.putBytes(blockId, new ChunkedByteBuffer(dataRead.duplicate()), storageLevel, |
There was a problem hiding this comment.
Why encrypt should be true here? In the following codes, it just reads the block using maybeEncrypted = false.
There was a problem hiding this comment.
This is explained in the summary.
The code that uses maybeEncrypted = false is deserializing data read from the WAL. This code is adding the block to the block manager, which later is read with getBlockFromBlockManager which calls blockManager.get which does decryption automatically.
|
Looks good. |
|
Test build #72667 has finished for PR 16862 at commit
|
|
We can remove the For now, I'm merging this to master and 2.1. |
Spark's I/O encryption uses an ephemeral key for each driver instance. So driver B cannot decrypt data written by driver A since it doesn't have the correct key. The write ahead log is used for recovery, thus needs to be readable by a different driver. So it cannot be encrypted by Spark's I/O encryption code. The BlockManager APIs used by the WAL code to write the data automatically encrypt data, so changes are needed so that callers can to opt out of encryption. Aside from that, the "putBytes" API in the BlockManager does not do encryption, so a separate situation arised where the WAL would write unencrypted data to the BM and, when those blocks were read, decryption would fail. So the WAL code needs to ask the BM to encrypt that data when encryption is enabled; this code is not optimal since it results in a (temporary) second copy of the data block in memory, but should be OK for now until a more performant solution is added. The non-encryption case should not be affected. Tested with new unit tests, and by running streaming apps that do recovery using the WAL data with I/O encryption turned on. Author: Marcelo Vanzin <[email protected]> Closes #16862 from vanzin/SPARK-19520. (cherry picked from commit 0169360) Signed-off-by: Marcelo Vanzin <[email protected]>
Spark's I/O encryption uses an ephemeral key for each driver instance. So driver B cannot decrypt data written by driver A since it doesn't have the correct key. The write ahead log is used for recovery, thus needs to be readable by a different driver. So it cannot be encrypted by Spark's I/O encryption code. The BlockManager APIs used by the WAL code to write the data automatically encrypt data, so changes are needed so that callers can to opt out of encryption. Aside from that, the "putBytes" API in the BlockManager does not do encryption, so a separate situation arised where the WAL would write unencrypted data to the BM and, when those blocks were read, decryption would fail. So the WAL code needs to ask the BM to encrypt that data when encryption is enabled; this code is not optimal since it results in a (temporary) second copy of the data block in memory, but should be OK for now until a more performant solution is added. The non-encryption case should not be affected. Tested with new unit tests, and by running streaming apps that do recovery using the WAL data with I/O encryption turned on. Author: Marcelo Vanzin <[email protected]> Closes apache#16862 from vanzin/SPARK-19520.
4 participants
Spark's I/O encryption uses an ephemeral key for each driver instance.
So driver B cannot decrypt data written by driver A since it doesn't
have the correct key.
The write ahead log is used for recovery, thus needs to be readable by
a different driver. So it cannot be encrypted by Spark's I/O encryption
code.
The BlockManager APIs used by the WAL code to write the data automatically
encrypt data, so changes are needed so that callers can to opt out of
encryption.
Aside from that, the "putBytes" API in the BlockManager does not do
encryption, so a separate situation arised where the WAL would write
unencrypted data to the BM and, when those blocks were read, decryption
would fail. So the WAL code needs to ask the BM to encrypt that data
when encryption is enabled; this code is not optimal since it results
in a (temporary) second copy of the data block in memory, but should be
OK for now until a more performant solution is added. The non-encryption
case should not be affected.
Tested with new unit tests, and by running streaming apps that do
recovery using the WAL data with I/O encryption turned on.