Skip to content
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

Provide uniform way to handle DBS server errors #11176

Merged
merged 3 commits into from
Jun 21, 2022
Merged

Provide uniform way to handle DBS server errors #11176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
56b28b0
Fix issue #11167
vkuznet Jun 9, 2022
36a8314
Add unit test for parseDBSException
vkuznet Jun 9, 2022
b9c22ec
Comment out Proxy Error mocking
vkuznet Jun 17, 2022
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
24 changes: 17 additions & 7 deletions src/python/WMComponent/DBS3Buffer/DBSUploadPoller.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,25 +99,35 @@ def uploadWorker(workInput, results, dbsUrl, gzipEncoding=False):
logging.warning("Block %s already exists. Marking it as uploaded.", name)
logging.debug("Exception: %s", exString)
results.put({'name': name, 'success': "uploaded"})
elif 'Proxy Error' in exString:
# This is probably a successfully insertion that went bad.
# Put it on the check list
msg = "Got a proxy error for block %s." % name
logging.warning(msg)
results.put({'name': name, 'success': "check"})
elif 'Missing data when inserting to dataset_parents' in exString:
msg = "Parent dataset is not inserted yet for block %s." % name
logging.warning(msg)
results.put({'name': name, 'success': "error", 'error': msg})
else:
msg = "Error trying to process block %s through DBS. Error: %s" % (name, exString)
reason = parseDBSException(exString)
amaltaro marked this conversation as resolved.
Show resolved Hide resolved
msg = "Error trying to process block %s through DBS. Error: %s" % (name, reason)
logging.exception(msg)
logging.debug("block info: %s \n", block)
results.put({'name': name, 'success': "error", 'error': msg})

return


def parseDBSException(exBodyString):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi again @vkuznet. Sorry for reiterating through this upon PR approval from my side, but I started wondering if that function won't be better to be a separate method in a dedicated DBSExeption class. One possible place could be here: https://github.com/dmwm/WMCore/blob/master/src/python/WMCore/Services/DBS/DBSErrors.py or equivalent one in WMComponent/DBS3Buffer

Just an idea though, not requesting any changes here. If not accepted, the current approach also works perfectly fine according to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@todor-ivanov , if code is independent (as it is now) it is better to keep it in separated function and not part of any class. But I don't have strong opinion in which module it should reside. So far we use it only in DBSUploadPoller and not anywhere else. As such it is better to reside over there and be treated as local function. But if there is use-case to use it in different parts of WMCore then yes may be DBSErrors.py can be a better place to hold this function .

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @vkuznet
Agreed :) lets move on as it is right now. And if we find a use case we can always move it in the general exception class.

"""
parse DBS Go-based server exception
:param exBodyString: exception message body string (not exception).
The upstream code extract HTTP body from exception object and pass it here.
:return: either (parsed) concise exception message or original body string
"""
try:
data = json.loads(exBodyString)
# dbs2go always return a list
return data[0]['error']['reason']
except:
return exBodyString


def isPassiveError(exceptionObj):
"""
This function will parse the exception object and report whether
Expand Down
8 changes: 5 additions & 3 deletions src/python/WMQuality/Emulators/DBSClient/DBS3API.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,11 @@ def insertBulkBlock(self, blockDump):
currentInfo.append(blockDump)
json.dump(currentInfo, outFileHandle)

randomNumber = random()
if randomNumber < 0.2:
raise Exception("Proxy Error, this is a mock proxy error.")
# VK: once we introduced parseDBSException function we do not need to simulate Proxy Error
# see https://github.com/dmwm/WMCore/pull/11176#issuecomment-1158758332
# randomNumber = random()
# if randomNumber < 0.2:
# raise Exception("Proxy Error, this is a mock proxy error.")

return

Expand Down
17 changes: 16 additions & 1 deletion test/python/WMComponent_t/DBS3Buffer_t/DBSUpload_t.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from WMComponent.DBS3Buffer.DBSBufferDataset import DBSBufferDataset
from WMComponent.DBS3Buffer.DBSBufferFile import DBSBufferFile
from WMComponent.DBS3Buffer.DBSBufferUtil import DBSBufferUtil
from WMComponent.DBS3Buffer.DBSUploadPoller import DBSUploadPoller, isPassiveError
from WMComponent.DBS3Buffer.DBSUploadPoller import DBSUploadPoller, isPassiveError, parseDBSException
from WMCore.DAOFactory import DAOFactory
from WMCore.DataStructs.Run import Run
from WMCore.Services.UUIDLib import makeUUID
Expand Down Expand Up @@ -638,6 +638,21 @@ def testPassiveExceptions(self):
'OpenSSL SSL_connect: SSL_ERROR_SYSCALL']:
self.assertIs(isPassiveError(MyTestException(errMessage)), True)

def testParseDBSException(self):
vkuznet marked this conversation as resolved.
Show resolved Hide resolved
"""
test parseDBSException function to parse DBS Go-based server exception message
"""
exc = """[{"error":{"reason":"DBSError Code:110 Description:DBS DB insert record error Function:dbs.bulkblocks.insertFilesViaChunks Message: Error: concurrency error","message":"7cf3dee6307fdaa1f72d0dc03551fd0e6665f32114d752c37819c238cef7a231 unable to insert files, error DBSError Code:110 Description:DBS DB insert record error Function:dbs.bulkblocks.insertFilesViaChunks Message: Error: concurrency error","function":"dbs.bulkblocks.InsertBulkBlocksConcurrently","code":110},"http":{"method":"POST","code":400,"timestamp":"2022-05-27 19:21:45.67710595 +0000 UTC m=+191328.489143316","path":"/dbs/prod/global/DBSWriter/bulkblocks","user_agent":"DBSClient/Unknown/","x_forwarded_host":"dbs-prod.cern.ch","x_forwarded_for":"137.138.157.32","remote_addr":"137.138.63.204:39950"},"exception":400,"type":"HTTPError","message":"DBSError Code:110 Description:DBS DB insert record error Function:dbs.bulkblocks.InsertBulkBlocksConcurrently Message:7cf3dee6307fdaa1f72d0dc03551fd0e6665f32114d752c37819c238cef7a231 unable to insert files, error DBSError Code:110 Description:DBS DB insert record error Function:dbs.bulkblocks.insertFilesViaChunks Message: Error: concurrency error Error: nested DBSError Code:110 Description:DBS DB insert record error Function:dbs.bulkblocks.insertFilesViaChunks Message: Error: concurrency error"}]"""
data = parseDBSException(exc)
expect = 'DBSError Code:110 Description:DBS DB insert record error Function:dbs.bulkblocks.insertFilesViaChunks Message: Error: concurrency error'
self.assertEqual(data, expect)

# test another type of exception
exc = Exception()
data = parseDBSException(exc)
self.assertEqual(data, exc)
self.assertIs(isinstance(exc, Exception), True)


if __name__ == '__main__':
unittest.main()