HeartSaVioR commented on a change in pull request #28859:
URL: https://github.com/apache/spark/pull/28859#discussion_r444811210
##########
File path:
core/src/test/scala/org/apache/spark/deploy/history/HistoryServerDiskManagerSuite.scala
##########
@@ -158,4 +158,35 @@ class HistoryServerDiskManagerSuite extends SparkFunSuite
with BeforeAndAfter {
assert(manager.approximateSize(50L, true) > 50L)
}
+ test("SPARK-32024: update ApplicationStoreInfo.size during initializing") {
+ val manager = mockManager()
+ val leaseA = manager.lease(2)
+ doReturn(3L).when(manager).sizeOf(meq(leaseA.tmpPath))
+ val dstA = leaseA.commit("app1", None)
+ assert(manager.free() === 0)
+ assert(manager.committed() === 3)
+ // Simulate: service restarts, new disk manager (manager1) is initialized.
+ val manager1 = mockManager()
+ // Simulate: leveldb compaction before restart, directory size reduces.
+ doReturn(2L).when(manager1).sizeOf(meq(dstA))
+ doReturn(2L).when(manager1).sizeOf(meq(new File(testDir, "apps")))
+ manager1.initialize()
+ assert(manager1.free() === 1)
+ // If "ApplicationStoreInfo.size" is not correctly updated,
"IllegalStateException"
+ // would be thrown.
+ val leaseB = manager1.lease(2)
+ assert(manager1.free() === 1)
+ doReturn(2L).when(manager1).sizeOf(meq(leaseB.tmpPath))
+ val dstB = leaseB.commit("app2", None)
+ assert(manager1.committed() === 2)
+ val manager2 = mockManager()
+ // Simulate: cache entities are written after replaying, directory size
increases.
+ doReturn(3L).when(manager2).sizeOf(meq(dstB))
+ doReturn(3L).when(manager2).sizeOf(meq(new File(testDir, "apps")))
+ manager2.initialize()
+ assert(manager2.free() === 0)
+ val leaseC = manager2.lease(2)
Review comment:
Better to explicitly mention it leads eviction on cached entities, hence
free() goes to 1.
##########
File path:
core/src/test/scala/org/apache/spark/deploy/history/HistoryServerDiskManagerSuite.scala
##########
@@ -158,4 +158,35 @@ class HistoryServerDiskManagerSuite extends SparkFunSuite
with BeforeAndAfter {
assert(manager.approximateSize(50L, true) > 50L)
}
+ test("SPARK-32024: update ApplicationStoreInfo.size during initializing") {
+ val manager = mockManager()
+ val leaseA = manager.lease(2)
+ doReturn(3L).when(manager).sizeOf(meq(leaseA.tmpPath))
+ val dstA = leaseA.commit("app1", None)
+ assert(manager.free() === 0)
+ assert(manager.committed() === 3)
Review comment:
Could we track and verify ApplicationStoreInfo in store as well? I see
the test represents how HistoryServerDiskManager fails without the patch, but
it would be pretty much intuitive if we also verify what we've changed directly.
##########
File path:
core/src/test/scala/org/apache/spark/deploy/history/HistoryServerDiskManagerSuite.scala
##########
@@ -158,4 +158,35 @@ class HistoryServerDiskManagerSuite extends SparkFunSuite
with BeforeAndAfter {
assert(manager.approximateSize(50L, true) > 50L)
}
+ test("SPARK-32024: update ApplicationStoreInfo.size during initializing") {
+ val manager = mockManager()
+ val leaseA = manager.lease(2)
+ doReturn(3L).when(manager).sizeOf(meq(leaseA.tmpPath))
+ val dstA = leaseA.commit("app1", None)
+ assert(manager.free() === 0)
+ assert(manager.committed() === 3)
+ // Simulate: service restarts, new disk manager (manager1) is initialized.
+ val manager1 = mockManager()
+ // Simulate: leveldb compaction before restart, directory size reduces.
+ doReturn(2L).when(manager1).sizeOf(meq(dstA))
+ doReturn(2L).when(manager1).sizeOf(meq(new File(testDir, "apps")))
+ manager1.initialize()
+ assert(manager1.free() === 1)
+ // If "ApplicationStoreInfo.size" is not correctly updated,
"IllegalStateException"
+ // would be thrown.
+ val leaseB = manager1.lease(2)
+ assert(manager1.free() === 1)
+ doReturn(2L).when(manager1).sizeOf(meq(leaseB.tmpPath))
+ val dstB = leaseB.commit("app2", None)
+ assert(manager1.committed() === 2)
+ val manager2 = mockManager()
Review comment:
Let's add some empty lines in overall to bring readability.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
