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.

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

fix bug:There is a memory leak in this part of the code #89

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

orsline
Copy link

@orsline orsline commented Feb 13, 2023

The image cache is applied in the lib library for transferring images, which needs to be released, otherwise there will be a memory leak

@kubeedge-bot
Copy link
Collaborator

Welcome @orsline! It looks like this is your first PR to kubeedge/mappers-go 🎉

@kubeedge-bot kubeedge-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Feb 13, 2023
@RyanZhaoXB
Copy link
Collaborator

please recommit it with git commit -s to add your sign

@orsline orsline closed this Feb 14, 2023
@orsline
Copy link
Author

orsline commented Feb 14, 2023

renew the PR

@orsline orsline reopened this Feb 14, 2023
@orsline orsline closed this Feb 14, 2023
@orsline orsline reopened this Feb 14, 2023
@RyanZhaoXB
Copy link
Collaborator

it is still not signed properly. please git commit -s --amend to recommit it.

The image cache is applied in the lib library for transferring images, which needs to be released, otherwise there will be a memory leak
2.Modify data update strategy
When the device's desire value is inconsistent with the report value, edgecore will re-deliver the desire value. This judgment will cause the desire value to fail to be delivered, because the desire value saved by the mapper has not changed.

Signed-off-by: zhangyanhua <[email protected]>
@RyanZhaoXB
Copy link
Collaborator

please approve running workflows @fisherxu . thanks.

Copy link
Member

@fisherxu fisherxu left a comment

Choose a reason for hiding this comment

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

/approve
/assign @RyanZhaoXB

@kubeedge-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fisherxu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubeedge-bot kubeedge-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 15, 2023
@kubeedge-bot kubeedge-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 14, 2023
@@ -43,10 +43,6 @@ func SyncInfo(dic *di.Container, message mqtt.Message) {
if i == len(deviceInstances[instanceID].Twins) {
continue
}
// Desired value is not changed.
if deviceInstances[instanceID].Twins[i].Desired.Value == twinValue {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why delete this line?
if the desired value is not changed, we should set the value to device.

Copy link
Author

Choose a reason for hiding this comment

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

The logic processing in this place is not suitable for control message processing, such as controlling the camera to take pictures, one shot is fine, but two shots are not.
Currently, kubeedge does not support independent control messages, so the limitation of this place is removed first.

Copy link
Contributor

Choose a reason for hiding this comment

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

If removed, whether it will affect the original processing logic? Can we add some new logic to handle the situation you mentioned?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants