You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by Skye Wanderman-Milne <sk...@cloudera.com> on 2012/10/12 01:36:12 UTC

Review Request: patch for ZOOKEEPER-1513: "Unreasonable length" exception while starting a server.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7557/
-----------------------------------------------------------

Review request for zookeeper, Patrick Hunt, Henry Robinson, and Camille Fournier.


Description
-------

This change creates a new system property, zookeeper.maxDataSize, and modifies PrepRequestProcessor.pRequest2Txn and SerializeUtils.deserializeTxn to check a txn's data against this property. I left jute.maxbuffer and its associated buffer length checks in ServerCnxn, BinaryInputArchive, and ClientCnxn intact -- while these checks do not correctly measure data length (leading to ZK-1513) since they are looking at either raw on-disk records or TCP requests, they do provide a sanity check against allocating a huge buffer and depleting the heap (e.g., a corrupt request could indicate that it has a length of two gigabytes. This shows up in CRCTest.getCheckSum). I set zookeeper.maxDataSize to ~1MB and jute.maxbuffer to ~2MB so that the data check will fire before the buffer check (a multi txn full of huge txns could mess this up though).

I'm not sure if this patch is actually a good idea since it's changing a lot of stuff for a relatively minor sanity check. OTOH, while it increases code complexity, I think it makes it easier to understand what's going on and will make debugging easier (it spells out that there's a difference between buffer size and data size, you'll be able to tell when you're generating huge data vs. corrupt requests, etc.). If this is too complicated, a quick but confusing/ugly fix to ZK-1513 would be to pad jute.maxbuffer when reading from disk but not when making/receiving requests.


This addresses bug ZOOKEEPER-1513.
    https://issues.apache.org/jira/browse/ZOOKEEPER-1513


Diffs
-----

  docs/zookeeperAdmin.html 530c96c 
  src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml 47190a8 
  src/java/main/org/apache/jute/BinaryInputArchive.java 8f329eb 
  src/java/main/org/apache/zookeeper/ZooKeeper.java 4a16dd3 
  src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java b11db7c 
  src/java/main/org/apache/zookeeper/server/util/SerializeUtils.java f9647c4 

Diff: https://reviews.apache.org/r/7557/diff/


Testing
-------

ant test

I manually tested creating/setting nodes above/below/equal to the maxbuffer limit, and also starting a server with create/set records above/below/equal to the limit.


Thanks,

Skye Wanderman-Milne