Skip to content

Revert separation of polling threads, process queue singly#82

Merged
patrick-austin merged 3 commits into36_queuingfrom
75_revert_separation
Apr 23, 2025
Merged

Revert separation of polling threads, process queue singly#82
patrick-austin merged 3 commits into36_queuingfrom
75_revert_separation

Conversation

@patrick-austin
Copy link

Closes #75

queuedDownload.setStatus(DownloadStatus.PREPARING);
prepareDownload(queuedDownload, null, getQueueSessionId(sessionIds, queuedDownload.getFacilityName()));
}
logger.trace("Preparing 1 out of {} queued downloads", queueSize);

Choose a reason for hiding this comment

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

Unless I'm missing something, I think it would be good to have this at info level. It's only going to be output when a queued item is being moved out of the queue (I think) so it would be useful to be able to trace which bit of logic is taking it out of the queue.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah when this was trying to start all the Downloads it could (rather than one at a time) we logged before iterating over the list, which may have been empty. Which meant logging even when there was no work to do and we demoted this to trace. Now, because we return early if the list is empty, we only log when there is work to do so can increase this back to info I think.

prepareDownload(queuedDownload, null, getQueueSessionId(sessionIds, queuedDownload.getFacilityName()));
} else {
logger.trace("Preparing up to {} queued downloads", availableDownloads);
logger.trace("Preparing 1 out of {} queued downloads as {} spaces available", queueSize, availableDownloads);

Choose a reason for hiding this comment

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

Likewise I think this would be useful at info level

@patrick-austin
Copy link
Author

I've also made some changes to the datagateway_admin script. Option 3 was broken (wrong request type, parameter type and was re-using the same function name), and decided that we should be printing the status code and text for all responses (some were doing this already, but it was impossible to debug with how it was before since there's no exception raised, just a non-200 code which is never returned to the user.

@patrick-austin patrick-austin merged commit ab648e4 into 36_queuing Apr 23, 2025
1 check passed
@patrick-austin patrick-austin deleted the 75_revert_separation branch April 23, 2025 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants