Skip to content

feat(service): add memory limit to batch size#132

Merged
vitalymelni merged 7 commits intoMapColonies:masterfrom
vitalymelni:feature/use-batches-updating-tiles
Jan 1, 2024
Merged

feat(service): add memory limit to batch size#132
vitalymelni merged 7 commits intoMapColonies:masterfrom
vitalymelni:feature/use-batches-updating-tiles

Conversation

@vitalymelni
Copy link
Copy Markdown
Contributor

Question Answer
Bug fix
New feature
Breaking change
Deprecations
Documentation
Tests added
Chore

Further information:
Adds a configuration to limit the amount of bytes of target tiles in the memory, writing the tiles in batches to the target

@shimoncohen shimoncohen requested review from almog8k, asafMasa and shimoncohen and removed request for almog8k December 21, 2023 11:11
@shimoncohen shimoncohen added the enhancement New feature or request label Dec 21, 2023
@shimoncohen shimoncohen changed the title adds logic that will save the target tiles in batches, where batch size defined by bytes count feat(service): add memory limit to batch size Dec 21, 2023
Copy link
Copy Markdown
Contributor

@asafMasa asafMasa left a comment

Choose a reason for hiding this comment

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

Looking good

@vitalymelni vitalymelni force-pushed the feature/use-batches-updating-tiles branch from a5cf7cc to 1ff0f75 Compare January 1, 2024 07:40
@vitalymelni vitalymelni requested a review from asafMasa January 1, 2024 07:41
@vitalymelni vitalymelni requested a review from asafMasa January 1, 2024 11:25

using (this._activitySource.StartActivity("saving tiles"))
{
this._logger.LogInformation($"[{methodName}] Update {tiles.Count} tiles");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

replcae with this._logger.LogInformation($"[{methodName}] Updating tiles chunk Job: {task.JobId}, Task: {task.Id}"); and remove 2 redundent logs before the function

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

{
this._logger.LogInformation($"[{methodName}] Update {tiles.Count} tiles");
target.UpdateTiles(tiles);
this._logger.LogDebug($"[{methodName}] UpdateRelativeProgress");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

move this log inside "UpdateRelativeProgress" method

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

@vitalymelni vitalymelni merged commit 3050828 into MapColonies:master Jan 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants