DM-46430: Garbage collection of the completed table contribution requests #870
Conversation
| @@ -128,8 +128,8 @@ json IngestHttpSvcMod::_asyncCancelRequest() const { | |||
| checkApiVersion(__func__, 13); | |||
|
|
|||
| auto const id = stoul(params().at("id")); | |||
| auto const contrib = _ingestRequestMgr->cancel(id); | |||
| return json::object({{"contrib", contrib.toJson()}}); | |||
| [[maybe_unused]] bool const cancelled = _ingestRequestMgr->cancel(id); | |||
There was a problem hiding this comment.
cancelled is certainly not used here, so I'm not sure why it isn't just not defined.
There was a problem hiding this comment.
Sometimes it's useful (say, when one is debugging the application) to know if an element was removed or not. The return value is also meant to be used on the unit test... which I overlooked extending to check for the returned values of the method to ensure the method behaves correctly. I will fix the unit test. Thank you for noticing this!
There was a problem hiding this comment.
I was wrong - the new version of the unit test is already testing the completion status of the method.
| @@ -158,7 +158,8 @@ json IngestHttpSvcMod::_asyncTransCancelRequests() const { | |||
| json contribsJson = json::array(); | |||
| for (auto& contrib : contribs) { | |||
| try { | |||
| contribsJson.push_back(_ingestRequestMgr->cancel(contrib.id).toJson()); | |||
| [[maybe_unused]] bool const cancelled = _ingestRequestMgr->cancel(contrib.id); | |||
There was a problem hiding this comment.
Same as other maybe_unused insance.
| } | ||
| throw IngestRequestNotFound(context_ + string(__func__) + " request " + to_string(id) + " was not found"); | ||
| // Nn such request found, or the request was already completed. |
| * - The method may also throw exception should any problem happened while | ||
| * the method using other services (the Replication system's database, etc.). | ||
| * | ||
| * @param id The unique identifier of a request to be cancelled. | ||
| * @return The updated descriptor of the request. | ||
| * @throw IngestRequestNotFound If the request is unknown to the manager. | ||
| * @return 'true' if the request was found and successfully cancelled (or marked for cncellation). |
There was a problem hiding this comment.
cncellation -> cancellation
The output collection of the completed/cancalled/failed requests has been eliminated to avoid accumilating memory over the lifetime of the worker service. Minor changes to the request processing manager habe been made. The unit test was adjusted accordingly. The changed do not affet clients of the Replication/Ingest system's REST API.
iagaponenko
force-pushed
the
tickets/DM-46430
branch
from
5bcc90d
to
fa7c944
Compare
The output collection of the completed/cancalled/failed requests has been eliminated to avoid accumilating memory over the lifetime of the worker service. Minor changes to the request processing manager habe been made. The unit test was adjusted accordingly. The changed do not affet clients of the Replication/Ingest system's REST API.