Skip to content

log all db error opertions that don't update any rows #3185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
havetisyan merged 2 commits into from
Jan 31, 2026
+485 −12
Merged

log all db error opertions that don't update any rows #3185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6d0fd1b
log all db error opertions that don't update any rows
havetisyan Jan 30, 2026
cc35b97
add test cases to increase code coverage
havetisyan Jan 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion servers/zms/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
<packaging>war</packaging>

<properties>
<code.coverage.min>0.9745</code.coverage.min>
<code.coverage.min>0.9760</code.coverage.min>
</properties>

<dependencies>
Expand Down
66 changes: 55 additions & 11 deletions servers/zms/src/main/java/com/yahoo/athenz/zms/DBService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,11 +528,13 @@ boolean processPolicy(ObjectStoreConnection con, Policy originalPolicy, String d
if (newAssertions != null) {
for (Assertion assertion : newAssertions) {
if (!con.insertAssertion(domainName, policyName, policy.getVersion(), assertion)) {
LOG.error("unable to insert assertion for policy {} in domain {}", policyName, domainName);
return false;
}
// insert the new assertion conditions if any
if (assertion.getConditions() != null) {
if (!con.insertAssertionConditions(assertion.getId(), assertion.getConditions())) {
LOG.error("unable to insert assertion conditions for policy {} in domain {}", policyName, domainName);
Comment thread
havetisyan marked this conversation as resolved.
return false;
}
}
Expand Down Expand Up @@ -563,6 +565,7 @@ boolean processPolicy(ObjectStoreConnection con, Policy originalPolicy, String d
if (!ignoreDeletes) {
for (Assertion assertion : delAssertions) {
if (!con.deleteAssertion(domainName, policyName, policy.getVersion(), assertion.getId())) {
LOG.error("unable to delete assertion for policy {} in domain {}", policyName, domainName);
Comment thread
havetisyan marked this conversation as resolved.
return false;
}
}
Expand All @@ -571,10 +574,12 @@ boolean processPolicy(ObjectStoreConnection con, Policy originalPolicy, String d

for (Assertion assertion : addAssertions) {
if (!con.insertAssertion(domainName, policyName, policy.getVersion(), assertion)) {
LOG.error("unable to insert assertion for policy {} in domain {}", policyName, domainName);
return false;
}
if (assertion.getConditions() != null) {
if (!con.insertAssertionConditions(assertion.getId(), assertion.getConditions())) {
LOG.error("unable to insert assertion condition for policy {} in domain {}", policyName, domainName);
Comment thread
havetisyan marked this conversation as resolved.
return false;
}
}
Expand All @@ -588,6 +593,7 @@ boolean processPolicy(ObjectStoreConnection con, Policy originalPolicy, String d
for (Map.Entry<Long, List<AssertionCondition>> entry : delConditions.entrySet()) {
Long assertionId = entry.getKey();
if (!con.deleteAssertionConditions(assertionId)) {
LOG.error("unable to delete assertion condition for policy {} in domain {}", policyName, domainName);
Comment thread
havetisyan marked this conversation as resolved.
return false;
}
}
Expand All @@ -596,6 +602,7 @@ boolean processPolicy(ObjectStoreConnection con, Policy originalPolicy, String d
Long assertionId = entry.getKey();
List<AssertionCondition> conditionsList = entry.getValue();
if (!con.insertAssertionConditions(assertionId, new AssertionConditions().setConditionsList(conditionsList))) {
LOG.error("unable to insert assertion condition for policy {} in domain {}", policyName, domainName);
Comment thread
havetisyan marked this conversation as resolved.
return false;
}
}
Expand Down Expand Up @@ -819,6 +826,8 @@ boolean processRole(ObjectStoreConnection con, Role originalRole, String domainN
}
member.setRequestPrincipal(admin);
if (!con.insertRoleMember(domainName, roleName, member, admin, auditRef)) {
LOG.error("unable to insert member {} to role {} in domain {}",
member.getMemberName(), roleName, domainName);
return false;
}
}
Expand Down Expand Up @@ -922,6 +931,8 @@ boolean processGroup(ObjectStoreConnection con, Group originalGroup, final Strin
member.setPendingState(null);
}
if (!con.insertGroupMember(domainName, groupName, member, admin, auditRef)) {
LOG.error("unable to insert member {} to group {} in domain {}",
member.getMemberName(), groupName, domainName);
return false;
}
}
Expand Down Expand Up @@ -1155,11 +1166,15 @@ boolean processUpdateRoleMembers(ObjectStoreConnection con, Role originalRole,
if (pendingState && deleteProtection == Boolean.TRUE) {
member.setApproved(false).setPendingState(ZMSConsts.PENDING_REQUEST_DELETE_STATE);
if (!con.insertRoleMember(domainName, roleName, member, admin, auditRef)) {
LOG.error("unable to insert member {} to role {} in domain {}",
member.getMemberName(), roleName, domainName);
return false;
}
addMemberToNotifySet(notifyMembers, member.getMemberName());
} else {
if (!con.deleteRoleMember(domainName, roleName, member.getMemberName(), admin, auditRef)) {
LOG.error("unable to delete member {} from role {} in domain {}",
member.getMemberName(), roleName, domainName);
return false;
}
}
Expand All @@ -1175,6 +1190,8 @@ boolean processUpdateRoleMembers(ObjectStoreConnection con, Role originalRole,
member.setPendingState(null);
}
if (!con.insertRoleMember(domainName, roleName, member, admin, auditRef)) {
LOG.error("unable to insert member {} to role {} in domain {}",
member.getMemberName(), roleName, domainName);
return false;
}
}
Expand Down Expand Up @@ -1209,11 +1226,15 @@ private boolean processUpdateGroupMembers(ObjectStoreConnection con, Group origi
if (pendingState && deleteProtection == Boolean.TRUE) {
member.setApproved(false).setPendingState(ZMSConsts.PENDING_REQUEST_DELETE_STATE);
if (!con.insertGroupMember(domainName, groupName, member, admin, auditRef)) {
LOG.error("unable to insert member {} to group {} in domain {}",
member.getMemberName(), groupName, domainName);
return false;
}
addMemberToNotifySet(notifyMembers, member.getMemberName());
} else {
if (!con.deleteGroupMember(domainName, groupName, member.getMemberName(), admin, auditRef)) {
LOG.error("unable to delete member {} from group {} in domain {}",
member.getMemberName(), groupName, domainName);
return false;
}
}
Expand All @@ -1229,6 +1250,8 @@ private boolean processUpdateGroupMembers(ObjectStoreConnection con, Group origi
member.setPendingState(null);
}
if (!con.insertGroupMember(domainName, groupName, member, admin, auditRef)) {
LOG.error("unable to insert member {} to group {} in domain {}",
member.getMemberName(), groupName, domainName);
return false;
}
}
Expand Down Expand Up @@ -1290,6 +1313,7 @@ boolean processServiceIdentity(ResourceContext ctx, ObjectStoreConnection con, S

for (PublicKeyEntry publicKey : publicKeys) {
if (!con.insertPublicKeyEntry(domainName, serviceName, publicKey)) {
LOG.error("unable to insert public key to service {} in domain {}", serviceName, domainName);
Comment thread
havetisyan marked this conversation as resolved.
return false;
}
}
Expand Down Expand Up @@ -1331,6 +1355,7 @@ boolean processServiceIdentity(ResourceContext ctx, ObjectStoreConnection con, S
if (!ignoreDeletes) {
for (String publicKey : delPublicKeysSet) {
if (!con.deletePublicKeyEntry(domainName, serviceName, publicKey)) {
LOG.error("unable to delete public key from service {} in domain {}", serviceName, domainName);
Comment thread
havetisyan marked this conversation as resolved.
return false;
}
}
Expand All @@ -1339,13 +1364,15 @@ boolean processServiceIdentity(ResourceContext ctx, ObjectStoreConnection con, S

for (String publicKey : newPublicKeysSet) {
if (!con.insertPublicKeyEntry(domainName, serviceName, publicKeysMap.get(publicKey))) {
LOG.error("unable to insert public key to service {} in domain {}", serviceName, domainName);
Comment thread
havetisyan marked this conversation as resolved.
return false;
}
}
auditLogPublicKeyEntries(auditDetails, "added-publickeys", newPublicKeysSet, publicKeysMap);

for (String publicKey : updatePublicKeysSet) {
if (!con.updatePublicKeyEntry(domainName, serviceName, updatePublicKeysMap.get(publicKey))) {
LOG.error("unable to update public key in service {} in domain {}", serviceName, domainName);
Comment thread
havetisyan marked this conversation as resolved.
return false;
}
}
Expand Down Expand Up @@ -1374,13 +1401,15 @@ boolean processServiceIdentity(ResourceContext ctx, ObjectStoreConnection con, S

for (String host : delHosts) {
if (!con.deleteServiceHost(domainName, serviceName, host)) {
LOG.error("unable to delete host in service {} in domain {}", serviceName, domainName);
Comment thread
havetisyan marked this conversation as resolved.
return false;
}
}
auditLogStrings(auditDetails, "deleted-hosts", delHosts);

for (String host : newHosts) {
if (!con.insertServiceHost(domainName, serviceName, host)) {
LOG.error("unable to insert host to service {} in domain {}", serviceName, domainName);
Comment thread
havetisyan marked this conversation as resolved.
return false;
}
}
Expand Down Expand Up @@ -1587,13 +1616,15 @@ boolean processPolicyCopyAssertions(ObjectStoreConnection con, Policy policy, fi
// insert assertion (and get new assertion id)

if (!con.insertAssertion(domainName, policyName, version, assertion)) {
LOG.error("unable to insert assertion to policy {} in domain {}", policyName, domainName);
return false;
}

// copy assertion conditions for new assertion id

if (assertionConditions.getConditionsList() != null && !assertionConditions.getConditionsList().isEmpty()) {
if (!con.insertAssertionConditions(assertion.getId(), assertionConditions)) {
LOG.error("unable to insert assertion conditions to policy {} in domain {}", policyName, domainName);
Comment thread
havetisyan marked this conversation as resolved.
return false;
}
}
Expand Down Expand Up @@ -3255,8 +3286,7 @@ void executeDeleteUser(ResourceContext ctx, String userName, String domainName,
// left behind will be cleaned up from this operation

if (!con.deletePrincipal(userName, true)) {
throw ZMSUtils.notFoundError(caller + ": unable to delete user: "
+ userName, caller);
throw ZMSUtils.notFoundError(caller + ": unable to delete user: " + userName, caller);
}

// automatically update any domain contact record where this user is referenced
Expand Down Expand Up @@ -4539,6 +4569,8 @@ boolean processDomainContacts(ObjectStoreConnection con, final String domainName
for (Map.Entry<String, String> entry : updatedContacts.entrySet()) {
if (!StringUtil.isEmpty(entry.getValue())) {
if (!con.insertDomainContact(domainName, entry.getKey(), entry.getValue())) {
LOG.error("unable to insert contact {}/{} in domain {}",
entry.getKey(), entry.getValue(), domainName);
return false;
}
}
Expand All @@ -4552,6 +4584,7 @@ boolean processDomainContacts(ObjectStoreConnection con, final String domainName
if (updatedContacts.isEmpty()) {
for (String contact : originalContacts.keySet()) {
if (!con.deleteDomainContact(domainName, contact)) {
LOG.error("unable to delete contact type {} in domain {}", contact, domainName);
return false;
}
}
Expand All @@ -4568,17 +4601,20 @@ boolean processDomainContacts(ObjectStoreConnection con, final String domainName
if (!originalContacts.get(type).equals(name)) {
if (StringUtil.isEmpty(name)) {
if (!con.deleteDomainContact(domainName, type)) {
LOG.error("unable to delete contact {} in domain {}", type, domainName);
return false;
}
} else {
if (!con.updateDomainContact(domainName, type, name)) {
LOG.error("unable to update contact {}/{} in domain {}", type, name, domainName);
return false;
}
}
}
} else {
if (!StringUtil.isEmpty(entry.getValue())) {
if (!con.insertDomainContact(domainName, type, name)) {
LOG.error("unable to insert contact {}/{} in domain {}", type, name, domainName);
return false;
}
}
Expand All @@ -4591,6 +4627,7 @@ boolean processDomainContacts(ObjectStoreConnection con, final String domainName
for (String type : originalContacts.keySet()) {
if (!updatedContacts.containsKey(type)) {
if (!con.deleteDomainContact(domainName, type)) {
LOG.error("unable to delete contact type {} in domain {}", type, domainName);
return false;
}
}
Expand Down Expand Up @@ -7697,11 +7734,12 @@ private boolean insertRoleMembers(ResourceContext ctx, ObjectStoreConnection con
roleMember.setPendingState(roleMember.getApproved() == Boolean.FALSE ? ZMSConsts.PENDING_REQUEST_ADD_STATE : null);
roleMember.setRequestPrincipal(principal);
if (!con.insertRoleMember(domainName, roleName, roleMember, principal, auditRef)) {
LOG.error("unable to update member {}", roleMember.getMemberName());
LOG.error("unable to update member {} in domain {}", roleMember.getMemberName(), domainName);
continue;
}
} catch (Exception ex) {
LOG.error("unable to update member {} error: {}", roleMember.getMemberName(), ex.getMessage());
LOG.error("unable to update member {} in domain {}, error: {}", roleMember.getMemberName(),
domainName, ex.getMessage());
continue;
}

Expand Down Expand Up @@ -7729,11 +7767,12 @@ boolean insertGroupMembers(ResourceContext ctx, ObjectStoreConnection con, List<
for (GroupMember groupMember : groupMembers) {
try {
if (!con.insertGroupMember(domainName, groupName, groupMember, principal, auditRef)) {
LOG.error("unable to update group member {}", groupMember.getMemberName());
LOG.error("unable to update group member {} in domain {}", groupMember.getMemberName(), domainName);
continue;
}
} catch (Exception ex) {
LOG.error("unable to update member {} error: {}", groupMember.getMemberName(), ex.getMessage());
LOG.error("unable to update member {} in domain {}, error: {}", groupMember.getMemberName(),
domainName, ex.getMessage());
continue;
}

Expand Down Expand Up @@ -7762,11 +7801,12 @@ boolean updateRoleMemberDisabledState(ResourceContext ctx, ObjectStoreConnection
try {
if (!con.updateRoleMemberDisabledState(domainName, roleName, roleMember.getMemberName(), principal,
roleMember.getSystemDisabled(), auditRef)) {
LOG.error("unable to update member {}", roleMember.getMemberName());
LOG.error("unable to update member {} in domain {}", roleMember.getMemberName(), domainName);
continue;
}
} catch (Exception ex) {
LOG.error("unable to update member {} error: {}", roleMember.getMemberName(), ex.getMessage());
LOG.error("unable to update member {} in domain {}, error: {}", roleMember.getMemberName(),
domainName, ex.getMessage());
continue;
}

Expand Down Expand Up @@ -7795,11 +7835,12 @@ boolean updateGroupMemberDisabledState(ResourceContext ctx, ObjectStoreConnectio
try {
if (!con.updateGroupMemberDisabledState(domainName, groupName, groupMember.getMemberName(), principal,
groupMember.getSystemDisabled(), auditRef)) {
LOG.error("unable to update group member {}", groupMember.getMemberName());
LOG.error("unable to update group member {} in domain {}", groupMember.getMemberName(), domainName);
continue;
}
} catch (Exception ex) {
LOG.error("unable to update group member {} error: {}", groupMember.getMemberName(), ex.getMessage());
LOG.error("unable to update group member {} in domain {}, error: {}", groupMember.getMemberName(),
domainName, ex.getMessage());
continue;
}

Expand Down Expand Up @@ -9220,7 +9261,8 @@ void executePutAssertionConditions(ResourceContext ctx, String domainName, Strin
// process our insert assertion condition.

if (!con.insertAssertionConditions(assertionId, assertionConditions)) {
throw ZMSUtils.requestError(String.format("%s: unable to insert assertion conditions for policy=%s assertionId=%d", caller, policyName, assertionId), caller);
throw ZMSUtils.requestError(String.format("%s: unable to insert assertion conditions for policy=%s assertionId=%d",
caller, policyName, assertionId), caller);
}

// update our policy and domain time-stamps, and invalidate local cache entry
Expand Down Expand Up @@ -9573,6 +9615,7 @@ private boolean processDomainDependency(ObjectStoreConnection con, String domain
// if we didn't insert a dependency then we need to return failure

if (!con.insertDomainDependency(domainName, service)) {
LOG.error("unable to insert dependency {} in domain {}", service, domainName);
return false;
}

Expand All @@ -9592,6 +9635,7 @@ private boolean processDeleteDomainDependency(ObjectStoreConnection con, String
// if we didn't delete the dependency then we need to return failure

if (!con.deleteDomainDependency(domainName, service)) {
LOG.error("unable to delete dependency {} in domain {}", service, domainName);
return false;
}

Expand Down
Loading
Loading