jerryshao commented on code in PR #12823:
URL: https://github.com/apache/gravitino/pull/12823#discussion_r3947076128
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/FunctionMetaService.java:
##########
@@ -333,15 +313,171 @@ private void fillFunctionPOBuilderParentEntityId(
builder.withSchemaId(namespacedEntityId.entityId());
}
- private FunctionPO updateFunctionPO(FunctionPO oldFunctionPO, FunctionEntity
newFunction) {
- Integer newVersion = oldFunctionPO.functionLatestVersion() + 1;
+ private FunctionPO updateFunctionPO(
+ FunctionPO oldFunctionPO,
+ FunctionEntity newFunction,
+ Long newSchemaId,
+ Long newCatalogId,
+ Long newMetalakeId) {
+ // Both version columns always advance together, but deriving the next
version from
+ // the higher of the two keeps it above every version row that exists,
even if a
+ // future path ever leaves the current version behind the latest one.
+ Integer newVersion = nextVersion(oldFunctionPO);
FunctionPO.FunctionPOBuilder builder =
FunctionPO.builder()
- .withMetalakeId(oldFunctionPO.metalakeId())
- .withCatalogId(oldFunctionPO.catalogId())
- .withSchemaId(oldFunctionPO.schemaId())
+ .withMetalakeId(newMetalakeId)
+ .withCatalogId(newCatalogId)
+ .withSchemaId(newSchemaId)
.withFunctionLatestVersion(newVersion)
.withFunctionCurrentVersion(newVersion);
return buildFunctionPO(newFunction, builder, newVersion);
}
+
+ /**
+ * Writes a new function or replaces the active function selected by natural
key or stable ID.
+ * Locking the root before choosing the next version makes overwrite
behavior identical across
+ * databases and keeps standard reads strict about the matching version-row
invariant.
+ */
+ private void insertFunctionWithoutCommit(
+ FunctionEntity functionEntity, FunctionPO initializedFunctionPO, boolean
overwrite) {
+ if (!overwrite) {
+ insertNewFunctionWithoutCommit(initializedFunctionPO);
+ return;
+ }
+
+ FunctionPO existingFunctionPO =
findAndLockFunctionForOverwrite(initializedFunctionPO);
+ if (existingFunctionPO == null) {
+ insertNewFunctionWithoutCommit(initializedFunctionPO);
+ return;
+ }
+
+ FunctionPO replacementPO = functionPOForOverwrite(initializedFunctionPO,
existingFunctionPO);
+ int updated =
+ SessionUtils.getWithoutCommit(
+ FunctionMetaMapper.class,
+ mapper -> ops.updatePO(mapper, replacementPO, existingFunctionPO));
+ if (updated == 0) {
+ throw functionWriteFailure(functionEntity.nameIdentifier(),
existingFunctionPO);
+ }
+ SessionUtils.doWithoutCommit(
+ FunctionVersionMetaMapper.class,
+ mapper ->
mapper.insertFunctionVersionMeta(replacementPO.functionVersionPO()));
+ }
+
+ private void insertNewFunctionWithoutCommit(FunctionPO functionPO) {
+ SessionUtils.doWithoutCommit(
+ FunctionMetaMapper.class, mapper -> ops.insertPO(mapper, functionPO,
false));
+ SessionUtils.doWithoutCommit(
+ FunctionVersionMetaMapper.class,
+ mapper ->
mapper.insertFunctionVersionMeta(functionPO.functionVersionPO()));
+ }
+
+ private FunctionPO findAndLockFunctionForOverwrite(FunctionPO
initializedFunctionPO) {
Review Comment:
Minor, non-blocking: `findAndLockFunctionForOverwrite` (try a locking SELECT
by natural key, fall back to a locking SELECT by ID, reject the ID match if it
belongs to a different parent) is a near line-for-line copy of
`TagMetaService.findAndLockTagForOverwrite`.
Worth noting this is now the *third* independent implementation of this
overwrite-resolution algorithm in this OCC effort - `FilesetMetaService`'s
version (landed first) only does the name-based lookup with no ID fallback, and
never got updated when Tag's newer, more capable pattern was added. This PR
copies Tag's version into Function, again without extracting to
`OccWriteSupport`, which already has same-shaped generic helpers (e.g.
`lockParentForChildWrite`) this would fit naturally as something like
`findAndLockForOverwrite(byNameLookup, byIdLookup, sameParent)`.
Not blocking, but a future fix to this algorithm (including the race in my
other comment) will need to be found and applied separately across
Tag/Fileset/Function - Fileset already didn't receive Tag's improvement, so the
copies do drift in practice.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
