Skip to content

Commit

Permalink
Merge pull request #195 from tumblr/gabe-no-addresspool-cache
Browse files Browse the repository at this point in the history 
remove AddressPool cache
  • Loading branch information
@byxorna
byxorna committed Aug 21, 2014
2 parents 930ebc2 + 4d1dc26 commit 4e15fa4
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 178 deletions.
87 changes: 2 additions & 85 deletions app/models/IpAddresses.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ case class IpAddresses(
def toJsValue = Json.toJson(this)
}

object IpAddresses extends IpAddressStorage[IpAddresses] with IpAddressCacheManagement {
object IpAddresses extends IpAddressStorage[IpAddresses] {
import org.squeryl.PrimitiveTypeMode._

override protected def createEventName: Option[String] = Some("ipAddresses_create")
Expand Down Expand Up @@ -56,14 +56,7 @@ object IpAddresses extends IpAddressStorage[IpAddresses] with IpAddressCacheMana
val calc = IpAddressCalc(network, startAt)
val gateway: Long = getGateway().getOrElse(calc.minAddressAsLong)
val netmask: Long = calc.netmaskAsLong
val currentMax: Option[Long] = iteration match {
case norm if norm == 0 =>
nextAddressFromCache(scope)
case failed =>
logger.info("Address cache dirty. Failed cache lookup so using DB")
populateCacheIfNeeded(scope, true)
getCurrentLowestLocalMaxAddress(calc)
}
val currentMax: Option[Long] = getCurrentLowestLocalMaxAddress(calc)
val address: Long = calc.nextAvailableAsLong(currentMax)
(gateway, address, netmask)
}
Expand Down Expand Up @@ -148,49 +141,6 @@ object IpAddresses extends IpAddressStorage[IpAddresses] with IpAddressCacheMana
AddressConfig.flatMap(cfg => scope.flatMap(cfg.pool(_)).orElse(cfg.defaultPool))
}

protected def populateCacheIfNeeded(opool: Option[String], force: Boolean = false) {
opool.foreach { pool =>
AddressConfig.flatMap(_.pool(pool)).foreach { ap =>
if (!ap.hasAddressCache || force) {
logger.trace("populating address cache for pool %s".format(ap.name))
ap.clearAddresses
findInPool(pool).foreach { address =>
try ap.useAddress(address.address) catch {
case e =>
logger.info("Error using address %s in pool %s: %s".format(address.dottedAddress, pool, e.getMessage))
}
}
}
}
}
}

// NOTE if we find an unused address, and for some reason it is already in the DB, and the insert
// fails, we will recommend the exact same address for use. This happened when the
// ipAddresses_create callback wasn't working correctly. We need a way to detect that we're trying
// to use the same IP address
protected def nextAddressInPool(opool: Option[String]): Option[Long] = {
logger.trace("finding next address in pool %s".format(opool.getOrElse("UNKNOWN")))
opool.flatMap { pool =>
try {
logger.trace("found pool %s, looking for config".format(pool))
AddressConfig.flatMap(_.pool(pool)).map { ap =>
logger.debug("Next address in pool %s in cache is %s".format(ap.name, ap.nextDottedAddress))
ap.nextAddress - 1
}
} catch {
case e =>
logger.warn("Exception getting next address: %s".format(e))
None
}
}
}

protected def nextAddressFromCache(scope: Option[String]): Option[Long] = {
populateCacheIfNeeded(scope)
nextAddressInPool(scope)
}

protected[this] def generateFindQuery(addressRow: IpAddresses, address: String): LogicalBoolean = {
try {
if (address.split('.').size != 4) throw new Exception("Try again later")
Expand All @@ -212,36 +162,3 @@ object IpAddresses extends IpAddressStorage[IpAddresses] with IpAddressCacheMana
}
}

trait IpAddressCacheManagement { self: IpAddressStorage[IpAddresses] =>
// Callbacks to manage populating and managing the address caches
Callback.on("ipAddresses_create") { pce =>
val newAddress = pce.getNewValue.asInstanceOf[IpAddresses]
logger.debug("ipAddress_create pool %s".format(newAddress.pool))
IpAddresses.AddressConfig.flatMap(_.pool(newAddress.pool)).foreach { ap =>
logger.debug("Using address %s in pool %s".format(newAddress.dottedAddress, ap.name))
ap.useAddress(newAddress.address)
}
}
Callback.on("ipAddresses_update") { pce =>
val oldAddress = pce.getOldValue.asInstanceOf[IpAddresses]
val newAddress = pce.getNewValue.asInstanceOf[IpAddresses]
IpAddresses.AddressConfig.flatMap(_.pool(oldAddress.pool)).foreach { ap =>
logger.debug("Purging address %s from pool %s".format(oldAddress.dottedAddress, ap.name))
ap.unuseAddress(oldAddress.address)
}
IpAddresses.AddressConfig.flatMap(_.pool(newAddress.pool)).foreach { ap =>
logger.debug("Using address %s in pool %s".format(newAddress.dottedAddress, ap.name))
ap.useAddress(newAddress.address)
}
}
Callback.on("ipAddresses_delete") { pce =>
val oldAddress = pce.getOldValue.asInstanceOf[IpAddresses]
IpAddresses.AddressConfig.flatMap(_.pool(oldAddress.pool)).foreach { ap =>
logger.debug("Purging address %s from pool %s".format(oldAddress.dottedAddress, ap.name))
try ap.unuseAddress(oldAddress.address) catch {
case e =>
logger.info("Exception unusing old address %s: %s".format(oldAddress.address, e.getMessage))
}
}
}
}
37 changes: 0 additions & 37 deletions app/models/shared/AddressPool.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,46 +31,9 @@ case class AddressPool(
))
}

// Represent an IP range as a bit vector, where every address takes up a bit. We calculate the
// used addresses as VectorIndex + ipCalc.minAddressAsLong. We calculate the next available
// address as the next unused index >= the start address, plus the minAddress
private[this] val addressCache = LockingBitSet(ipCalc.addressCount)

def isInRange(address: String): Boolean = ipCalc.subnetInfo.isInRange(address)
def isInRange(address: Long): Boolean = isInRange(IpAddress.toString(address))

def clearAddresses() { addressCache.forWrite(_.clear()) }
def hasAddressCache(): Boolean = !addressCache.forRead(_.isEmpty)
def useAddress(address: String) {
useAddress(IpAddress.toLong(address))
}
def useAddress(address: Long) {
requireInRange(address)
addressCache.forWrite(_.set(addressIndex(address)))
}
def unuseAddress(address: String) {
unuseAddress(IpAddress.toLong(address))
}
def unuseAddress(address: Long) {
requireInRange(address)
addressCache.forWrite(_.set(addressIndex(address), false))
}
def nextDottedAddress(): String = {
IpAddress.toString(nextAddress)
}
def nextAddress(): Long = {
val idx = addressIndex(ipCalc.startAddressAsLong)
val next = addressCache.forRead(_.nextClearBit(idx))
val nextAddress = next + ipCalc.minAddressAsLong
requireInRange(nextAddress)
nextAddress
}
// mostly for testing
protected[shared] def usedDottedAddresses(): Set[String] = {
(for (i <- addressCache.indexIterator)
yield IpAddress.toString(addressFromIndex(i))).toSet
}

// Ensure as a set, address pools with the same name are seen as equal
override def equals(o: Any) = o match {
case that: AddressPool => that.name.equalsIgnoreCase(this.name)
Expand Down
103 changes: 47 additions & 56 deletions test/models/shared/AddressPoolSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,73 +7,64 @@ import org.specs2.matcher._

class AddressPoolSpec extends mutable.Specification {

class AddressPoolScope(network: String, startAddress: Option[String]) extends Scope {
val pool = AddressPool("TEST", network, startAddress, None)
def useAddress(address: String) = pool.useAddress(address)
def unuseAddress(address: String) = pool.unuseAddress(address)
def nextAddress(): String = pool.nextDottedAddress()
def useNext(n: Int) {
for (i <- 0 until n) pool.useAddress(nextAddress)
}
def usedAddressCount(): Int = pool.usedDottedAddresses.size
}

val DEFAULT_NET = "172.16.16.0/24"

"Address Pools" should {

"Support a default start address" in new AddressPoolScope(DEFAULT_NET, None) {
usedAddressCount === 0
nextAddress === "172.16.16.1"
useNext(10)
usedAddressCount === 10
nextAddress === "172.16.16.11"
unuseAddress("172.16.16.5")
usedAddressCount === 9
nextAddress === "172.16.16.5"
"Pool name must be all caps" in {
val good = AddressPool("TEST",DEFAULT_NET,None,None)
good.name === "TEST"
AddressPool("lowercasepool",DEFAULT_NET,None,None) must throwA[IllegalArgumentException]
}

"Error when given bogus networks" in {
AddressPool("BAD_NET1","",None,None) must throwA[IllegalArgumentException]
AddressPool("BAD_NET1","not a network",None,None) must throwA[IllegalArgumentException]
val p = AddressPool("GOOD_NET1","10.2.0.0/23",None,None)
p.network === "10.2.0.0/23"
}

"Start address should be a valid address" in {
val p = AddressPool("GOOD_START1","10.2.0.0/23",None,None)
p.name === "GOOD_START1"
AddressPool("BAD_START1","10.2.0.0/23",Some(""),None) must throwA[IllegalArgumentException]
AddressPool("BAD_START1","10.2.0.0/23",Some("bad start address"),None) must throwA[IllegalArgumentException]
}
"Support a specified start address" in new AddressPoolScope(DEFAULT_NET, Some("172.16.16.10")) {
usedAddressCount === 0
nextAddress === "172.16.16.10"
useAddress(nextAddress)
usedAddressCount === 1
nextAddress === "172.16.16.11"
unuseAddress("172.16.16.10")
usedAddressCount === 0
nextAddress === "172.16.16.10"

"Start address should be within network range" in {
val p = AddressPool("IN_RANGE1","10.2.0.0/23",Some("10.2.0.5"),None)
p.startAddress.get === "10.2.0.5"
AddressPool("OUT_OF_RANGE1","10.2.0.0/23",Some("192.168.1.1"),None) must throwA[IllegalArgumentException]
}

"Support sparse IP ranges" in new AddressPoolScope(DEFAULT_NET, Some("172.16.16.200")) {
usedAddressCount === 0
nextAddress === "172.16.16.200"
useNext(20)
usedAddressCount === 20
nextAddress === "172.16.16.220"
unuseAddress("172.16.16.215")
usedAddressCount === 19
nextAddress === "172.16.16.215"
"Support checking if address is in pool" in {
val p = AddressPool("TEST2","10.2.0.0/24",Some("10.2.0.5"),None)
p.isInRange("10.2.0.0") === false
p.isInRange("10.2.0.1") === true
p.isInRange("10.0.0.0") === false
p.isInRange("10.2.0.254") === true
p.isInRange("10.2.0.255") === false
p.isInRange("not an address") must throwA[IllegalArgumentException]
p.isInRange(util.IpAddress.toLong("10.2.0.12")) === true
p.isInRange(3232235777L) === false // 192.168.1.1
}

"Support using out of range IP addresses" in new AddressPoolScope(DEFAULT_NET, Some("172.16.16.10")) {
nextAddress === "172.16.16.10"
useAddress("172.16.16.2")
usedAddressCount === 1
nextAddress === "172.16.16.10"
useAddress(nextAddress)
usedAddressCount === 2
nextAddress === "172.16.16.11"
"Support comparing pools by name" in {
val p1 = AddressPool("P1","10.2.0.0/23",Some("10.2.0.5"),None)
val p2 = AddressPool("P1","192.168.1.0/24",None,None)
val p3 = AddressPool("P3","192.168.1.0/24",None,None)
p1.equals(p2) === true
p1.equals(p3) === false
}

"Misbehave if over allocating" in new AddressPoolScope("172.16.16.0/29", Some("172.16.16.5")) {
nextAddress === "172.16.16.5"
useAddress(nextAddress)
usedAddressCount === 1
nextAddress === "172.16.16.6"
useAddress(nextAddress)
usedAddressCount === 2
nextAddress must throwA[RuntimeException]
useAddress("172.16.16.7") must throwA[RuntimeException]
unuseAddress("10.10.10.10") must throwA[RuntimeException]
"Support comparing pools strictly" in {
val p1 = AddressPool("P1","10.2.0.0/23",Some("10.2.0.5"),None)
val p2 = AddressPool("P1","192.168.1.0/24",None,None)
val p3 = AddressPool("P1","10.2.0.0/23",Some("10.2.0.5"),None)
val p4 = AddressPool("P1","10.2.0.0/23",None,None)
p1.strictEquals(p2) === false
p1.strictEquals(p3) === true
p1.strictEquals(p4) === false
}

}
Expand Down

0 comments on commit 4e15fa4

Please sign in to comment.