Skip to content

Commit

Permalink
fix contract_tpid_fkey-related race condition (#11330)
Browse files Browse the repository at this point in the history
* trying to reliably reproduce the template ID constraint error

* tentative fix for template ID constraint error

* sequential simulation

* successfully reproduce the error pre-4633c3137a

- Batch entry 0
        INSERT INTO some_fancy_prefix_contract
        VALUES ('foo', 1, 'null'::jsonb, NULL, '{}'::jsonb, ?, ?, '')
        ON CONFLICT (contract_id) DO NOTHING
       was aborted: ERROR: insert or update on table "some_fancy_prefix_contract" violates foreign key constraint "some_fancy_prefix_contract_tpid_fkey"
  Detail: Key (tpid)=(1) is not present in table "some_fancy_prefix_template_id".

* also reproduced the error pre-4633c3137a on Oracle

- ORA-02291: integrity constraint (UNA3GOHUV7YMSKT0MQXJKLKD9HKKAZ.SYS_C007859)
  violated - parent key not found

* add changelog

CHANGELOG_BEGIN
- [JSON API] Fixed a rare error that manifested as
  ‘violates foreign key constraint "contract_tpid_fkey"
   Detail: Key (tpid)=(...) is not present in table’
  when attempting to run queries and goes away on JSON API restart.
  See `issue #11330 <https://github.com/digital-asset/daml/pull/11330>`__.
CHANGELOG_END

* clean up some now-unneeded printlns
  • Loading branch information
S11001001 authored Oct 22, 2021
1 parent ab8a863 commit 3bc0db3
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,10 @@ sealed abstract class Queries(tablePrefix: String, tpIdCacheMaxEntries: Long)(im
Applicative[ConnectionIO].pure(tpId)
}
.getOrElse {
surrogateTemplateIdFromDb(packageId, moduleName, entityName).map { tpId =>
for {
tpId <- surrogateTemplateIdFromDb(packageId, moduleName, entityName)
_ <- connection.commit
} yield {
surrogateTpIdCache.setCacheValue(packageId, moduleName, entityName, tpId)
tpId
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ import com.daml.http.util.Logging.{InstanceUUID, instanceUUIDLogCtx}
import com.daml.logging.LoggingContextOf
import com.daml.metrics.Metrics
import doobie.util.log.LogHandler
import doobie.free.{connection => fconn}
import org.scalatest.freespec.AsyncFreeSpecLike
import org.scalatest.{Assertion, AsyncTestSuite, BeforeAndAfterAll, Inside}
import org.scalatest.matchers.should.Matchers
import scalaz.std.list._

import scala.collection.compat._
import scala.concurrent.Future
Expand Down Expand Up @@ -184,25 +186,69 @@ abstract class AbstractDatabaseIntegrationTest extends AsyncFreeSpecLike with Be
} yield succeed
}.unsafeToFuture()

"SurrogateTemplateIdCache should be used on template insertion and reads" in {
import dao.jdbcDriver.q.queries
def getOrElseInsertTemplate(tpid: TemplateId[String])(implicit
logHandler: LogHandler = dao.logHandler
) = instanceUUIDLogCtx(implicit lc =>
dao.transact(
queries
.surrogateTemplateId(tpid.packageId, tpid.moduleName, tpid.entityName)
"SurrogateTemplateIdCache" - {
"should be used on template insertion and reads" in {
import dao.jdbcDriver.q.queries
def getOrElseInsertTemplate(tpid: TemplateId[String])(implicit
logHandler: LogHandler = dao.logHandler
) = instanceUUIDLogCtx(implicit lc =>
dao.transact(
queries
.surrogateTemplateId(tpid.packageId, tpid.moduleName, tpid.entityName)
)
)
)
val tpId = TemplateId("pkg", "mod", "ent")
for {
storedStpId <- getOrElseInsertTemplate(tpId) //insert the template id into the cache
cachedStpId <- getOrElseInsertTemplate(tpId) // should trigger a read from cache
} yield {
storedStpId shouldEqual cachedStpId
queries.surrogateTpIdCache.getHitCount shouldBe 1
queries.surrogateTpIdCache.getMissCount shouldBe 1

val tpId = TemplateId("pkg", "mod", "ent")
for {
storedStpId <- getOrElseInsertTemplate(tpId) //insert the template id into the cache
cachedStpId <- getOrElseInsertTemplate(tpId) // should trigger a read from cache
} yield {
storedStpId shouldEqual cachedStpId
queries.surrogateTpIdCache.getHitCount shouldBe 1
queries.surrogateTpIdCache.getMissCount shouldBe 1
}
}.unsafeToFuture()

"doesn't cache uncommitted template IDs" in {
import dbbackend.Queries.DBContract, spray.json.{JsObject, JsNull, JsValue},
spray.json.DefaultJsonProtocol._
import dao.logHandler, dao.jdbcDriver.q.queries

val tpId = TemplateId("pkg", "mod", "UncomCollision")

val simulation = instanceUUIDLogCtx { implicit lc =>
def stid =
queries.surrogateTemplateId(tpId.packageId, tpId.moduleName, tpId.entityName)

for {
_ <- queries.dropAllTablesIfExist
_ <- queries.initDatabase
_ <- fconn.commit
_ <- stid
_ <- fconn.rollback // as with when we conflict and retry
tpid <- stid
_ <- queries.insertContracts(
List(
DBContract(
contractId = "foo",
templateId = tpid,
key = JsNull: JsValue,
keyHash = None,
payload = JsObject(): JsValue,
signatories = Seq("Alice"),
observers = Seq.empty,
agreementText = "",
)
)
)
_ <- fconn.commit
} yield succeed
}

dao
.transact(simulation)
.unsafeToFuture()
}
}.unsafeToFuture()
}

}

0 comments on commit 3bc0db3

Please sign in to comment.