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

SetDirty called more than CHIP_IM_SERVER_MAX_NUM_DIRTY_SET times will result in data being lost... #16273

Closed
mrjerryjohns opened this issue Mar 16, 2022 · 1 comment · Fixed by #17417
Assignees
Labels

Comments

@mrjerryjohns
Copy link
Contributor

Problem

If we call SetDirty more than CHIP_IM_SERVER_MAX_NUM_DIRTY_SET times synchronously in a given execution context, the remaining calls will fail without any real recovery strategy. This will result in those items not being marked dirty and failing to be delivered to clients, resulting in clients seeing a different view of data than what is on the server.

Proposed Solution

The crappiest answer here is to tear down all subscriptions since we can no longer guarantee coherency. We really should endeavor to do better than this.

Possible alternatives include some kind of coalescing/compaction of the dirty list that progressively lowers the precision of the path set to just free up a single slot:

  1. Do a sweep over the dirty set to see if we can find attributes that are part of the same cluster + endpoint, and if so, merge them together to a single path that points to the cluster/endpoint.
  2. If 1) doesn't yield a slot, then do a sweep over attributes on clusters that are part of the same endpoint, and if so, merge them together to a single path that points to the endpoint.
  3. If 2) doesn't yield a slot, then replace all paths with a single path that is wildcard endpoint and cluster.

The algorithm above should exit the moment it manages to free up a single slot.

@erjiaqing
Copy link
Contributor

erjiaqing commented Apr 15, 2022

I'm implemeting a almost same algorithm, however, I merge the existing paths before insert the new path.

Consider the following case:

The cap of ObjectPool is 8, and we have 8 paths of different attributes under the same cluster, then SetDirty is called with an attribute on another endpoint.

The proposed algorithm will yield a wildcard endpoint at last, which is too early for this case.

So I implement this by

  1. sweep the existing path to merge the attributes under the same cluster
  2. If 1 failed to yield a new slot, merge the existing attributes under the same endpoint
  3. If 2 failed to yield a new slot, replace the dirty set by wildcard endpoint.

Check if the new path can be merged into the existing paths after merging the existing paths.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants