Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/20011#discussion_r158514239
--- Diff:
core/src/test/scala/org/apache/spark/deploy/history/DiskStoreManagerSuite.scala
---
@@ -0,0 +1,161 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.deploy.history
+
+import java.io.File
+
+import org.mockito.Matchers.{any, anyBoolean, anyLong, eq => meq}
+import org.mockito.Mockito._
+import org.mockito.invocation.InvocationOnMock
+import org.mockito.stubbing.Answer
+import org.scalatest.BeforeAndAfter
+
+import org.apache.spark.{SparkConf, SparkFunSuite}
+import org.apache.spark.status.KVUtils
+import org.apache.spark.util.{ManualClock, Utils}
+import org.apache.spark.util.kvstore.KVStore
+
+class DiskStoreManagerSuite extends SparkFunSuite with BeforeAndAfter {
+
+ import config._
+
+ private val MAX_USAGE = 3L
+
+ private var testDir: File = _
+ private var store: KVStore = _
+
+ before {
+ testDir = Utils.createTempDir()
+ store = KVUtils.open(new File(testDir, "listing"), "test")
+ }
+
+ after {
+ store.close()
+ if (testDir != null) {
+ Utils.deleteRecursively(testDir)
+ }
+ }
+
+ private def mockManager(): DiskStoreManager = {
+ val conf = new SparkConf().set(MAX_LOCAL_DISK_USAGE, MAX_USAGE)
+ val manager = spy(new DiskStoreManager(conf, testDir, store, new
ManualClock()))
+ doReturn(0L).when(manager).sizeOf(any(classOf[File]))
+ doAnswer(new Answer[Long] {
+ def answer(invocation: InvocationOnMock): Long = {
+ invocation.getArguments()(0).asInstanceOf[Long]
+ }
+ }).when(manager).approximateSize(anyLong(), anyBoolean())
+ manager
+ }
+
+ private def hasFreeSpace(manager: DiskStoreManager, size: Long): Boolean
= {
+ size <= manager.free()
+ }
+
+ test("leasing space") {
+ val manager = mockManager()
+
+ // Lease all available space.
+ val lease1 = manager.lease(1)
+ val lease2 = manager.lease(1)
+ val lease3 = manager.lease(1)
+ assert(!hasFreeSpace(manager, 1))
+
+ // Revert one lease, get another one.
+ lease1.rollback()
+ assert(hasFreeSpace(manager, 1))
+ assert(!lease1.path.exists())
+
+ val lease4 = manager.lease(1)
+ assert(!hasFreeSpace(manager, 1))
+
+ // Committing 2 should bring the "used" space up to 4, so there
shouldn't be space left yet.
+ doReturn(2L).when(manager).sizeOf(meq(lease2.path))
+ val dst2 = lease2.commit("app2", None)
+ assert(!hasFreeSpace(manager, 1))
+
+ // Rollback 3 and 4, now there should be 1 left.
+ lease3.rollback()
+ lease4.rollback()
+ assert(hasFreeSpace(manager, 1))
+ assert(!hasFreeSpace(manager, 2))
--- End diff --
nit: I don't think `hasFreeSpace` actually helps at all, here you could
replace both of these with `assert(manager.free() === 1)` and everywhere else
do `assert(manager.free() === 0)` etc.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
