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

Memory leak in Cache indices when item is deleted #3510

Open
wuxudong opened this issue Jun 25, 2024 · 2 comments
Open

Memory leak in Cache indices when item is deleted #3510

wuxudong opened this issue Jun 25, 2024 · 2 comments
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines.

Comments

@wuxudong
Copy link

wuxudong commented Jun 25, 2024

Describe the bug

private void deleteFromIndices(ApiType oldObj, String key) {
    for (Map.Entry<String, Function<ApiType, List<String>>> indexEntry : this.indexers.entrySet()) {
      Function<ApiType, List<String>> indexFunc = indexEntry.getValue();
      List<String> indexValues = indexFunc.apply(oldObj);
      if (CollectionUtils.isEmpty(indexValues)) {
        continue;
      }

      Map<String, Set<String>> index = this.indices.get(indexEntry.getKey());
      if (index == null) {
        continue;
      }
      for (String indexValue : indexValues) {
        Set<String> indexSet = index.get(indexValue);
        if (indexSet != null) {
          indexSet.remove(key);
        }
      }
    }
  }

indexSet should also be deleted if it is empty; otherwise, a lot of untouchable HashSets will cause memory leaks.

The client-go fix is kubernetes/kubernetes#84959

A proper fix would be:

private void deleteFromIndices(ApiType oldObj, String key) {
    for (Map.Entry<String, Function<ApiType, List<String>>> indexEntry : this.indexers.entrySet()) {
      Function<ApiType, List<String>> indexFunc = indexEntry.getValue();
      List<String> indexValues = indexFunc.apply(oldObj);
      if (CollectionUtils.isEmpty(indexValues)) {
        continue;
      }

      Map<String, Set<String>> index = this.indices.get(indexEntry.getKey());
      if (index == null) {
        continue;
      }
      for (String indexValue : indexValues) {
        Set<String> indexSet = index.get(indexValue);
        if (indexSet != null) {
          indexSet.remove(key);
          if (indexSet.isEmpty()) {
              index.remove(indexValue);
          }
        }
      }
    }
  }

Client Version
15.0.1

Kubernetes Version
1.19.3

Java Version
Java 8

To Reproduce

Memory grows higher after items are created and removed.

Expected behavior

no memory leak after item is removed.

KubeConfig
If applicable, add a KubeConfig file with secrets redacted.

Server (please complete the following information):

  • OS: [e.g. Linux]
  • Environment [e.g. container]
  • Cloud [e.g. Azure]

Additional context
Add any other context about the problem here.

@wuxudong
Copy link
Author

Compare to client-go, in thread_safe_store.go

func (i *storeIndex) deleteKeyFromIndex(key, indexValue string, index Index) {
	set := index[indexValue]
	if set == nil {
		return
	}
	set.Delete(key)
	// If we don't delete the set when zero, indices with high cardinality
	// short lived resources can cause memory to increase over time from
	// unused empty sets. See `kubernetes/kubernetes/issues/84959`.
	if len(set) == 0 {
		delete(index, indexValue)
	}
}

@brendandburns
Copy link
Contributor

Makes sense, feel free to send a PR if you'd like. Otherwise we'll get to it eventually.

@brendandburns brendandburns added the good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. label Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines.
Projects
None yet
Development

No branches or pull requests

2 participants