Implicit transaction isolation levels in `DirectoryServiceImpl` can result in erroneous responses

[lberrymage]

Description

The default transaction isolation level (Read Committed) used in DirectoryServiceImpl can cause some of its gRPC service methods to return erroneous responses if database updates occur between queries. For example, in the following function:

directory/directory/src/main/kotlin/app/accrescent/server/directory/DirectoryServiceImpl.kt

Lines 72 to 99 in 0d0bbd2

	@WithSession
	override fun getAppListing(request: GetAppListingRequest): Uni<GetAppListingResponse> {
	validateRequestOrThrow(request)
	val response = findBestListingMatch(
	request.appId,
	request.preferredLanguagesList,
	).map { listing ->
	if (listing == null) {
	throw Status
	.fromCode(Status.Code.NOT_FOUND)
	.withDescription("app with ID ${request.appId} not found")
	.asRuntimeException()
	} else {
	val appListing = appListing {
	appId = listing.id.appId
	language = listing.id.language
	name = listing.name
	shortDescription = listing.shortDescription
	icon = image { url = "${artifactsBaseUrl}/${listing.icon.objectId}" }
	}
	getAppListingResponse { this.listing = appListing }
	}
	}
	return response
	}

the call to findBestListingMatch() results in multiple queries as can be seen here:

directory/directory/src/main/kotlin/app/accrescent/server/directory/DirectoryServiceImpl.kt

Lines 329 to 344 in 0d0bbd2

	private fun findBestListingMatch(
	appId: String,
	userPreferredLanguages: List<String>,
	): Uni<Listing?> {
	return Listing.findAllLanguagesForApp(appId).chain { listingLanguages ->
	val listingLanguages = listingLanguages.map { it.language }.toSet()
	for (preferredLanguage in userPreferredLanguages) {
	if (listingLanguages.contains(preferredLanguage)) {
	return@chain Listing.findByAppIdAndLanguage(appId, preferredLanguage)
	}
	}
	Listing.findDefaultForApp(appId)
	}
	}

Assume the following sequence occurs:

1. A client makes a call to GetAppListing() with preferred_languages set to "de", and the handler finishes calling Listing.findAllLanguagesForApp().
2. Immediately after Listing.findAllLanguagesForApp() returns ["en-US", "de"], the "de" listing for the app in question is deleted.
3. return@chain Listing.findByAppIdAndLanguage(appId, preferredLanguage) runs, where preferredLanguage is at this point "de".
4. The "de" listing does not exist, so GetAppListing() returns the NOT_FOUND status code even though the app exists.

There are other cases like this possible in some of the other gRPC service methods.

Resolution suggestions

We should almost certainly use the repeatable read transaction isolation level in PostgreSQL. According to the Postgres documentation, this guarantees the current transaction will not see any changes committed by concurrent transactions (note that this guarantee is not made in the SQL standard "repeatable read" isolation level, but is by Postgres).

Unfortunately, Quarkus doesn't seem to have any support for setting the transaction isolation level at any level more granular than per-datasource (see quarkusio/quarkus#17148). The best solution we seem to have is setting the transaction isolation level manually using native SQL, e.g.,

sessionFactory.withTransaction { session ->
    session
        .createNativeQuery<Unit>("SET TRANSACTION ISOLATION LEVEL REPEATABLE READ READ ONLY")
        .executeUpdate()
        .chain { ->
            // Insert actual queries here
            Uni.createFrom().voidItem()
        }
}