Skip to content

Implement registering for other memory notifications besides oom#154

Merged
crosbymichael merged 1 commit into
containerd:masterfrom
dcantah:v1_register_memory_events
Apr 7, 2020
Merged

Implement registering for other memory notifications besides oom#154
crosbymichael merged 1 commit into
containerd:masterfrom
dcantah:v1_register_memory_events

Conversation

@dcantah

@dcantah dcantah commented Mar 30, 2020

Copy link
Copy Markdown
Member
  • Implement functionality to register for events on the other memory cgroup files
    that support the notification API (memory.usage_in_bytes, memory.memsw.usage_in_bytes,
    memory.pressure_limit).

  • Small typo fix in hierarchy.go. enableds -> enables

Signed-off-by: Daniel Canter dcanter@microsoft.com

@crosbymichael

Copy link
Copy Markdown
Member

What do you think about a single method that can return uintptr, error but handle args and the type of event that you would like to receive? Just thinking so we could keep the interface small and support these and any other events from the subsystems in the future.

@dcantah

dcantah commented Mar 30, 2020

Copy link
Copy Markdown
Member Author

@crosbymichael I agree this would be better but I didn't know if you guys wanted to keep OOMEventFD as part of the interface so I simply added the new events to it as well. In your proposal we would be getting rid of OOMEventFD and the two new ones in favor of just one method (let's call it MemoryEvent or similar) if I'm understanding correctly?

@crosbymichael

Copy link
Copy Markdown
Member

I think we would need to keep OOMEventFD for a little bit to give ppl time to move over to the new method.

MemoryEvent sounds good to me.

@dcantah

dcantah commented Mar 30, 2020

Copy link
Copy Markdown
Member Author

@crosbymichael Cool I'll get to work

@dcantah dcantah changed the title Implement registering for other memory notifications besides oom [WIP] Implement registering for other memory notifications besides oom Mar 30, 2020
@crosbymichael

Copy link
Copy Markdown
Member

thanks!

* Implement funcs to register for events for the other memory cgroup files
that support the notification API (memory.usage_in_bytes, memory.memsw.usage_in_bytes,
memory.pressure_limit).

* Small typo fix in hierarchy.go.  enableds -> enables

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
@dcantah dcantah force-pushed the v1_register_memory_events branch from 0f13a87 to 0b707e8 Compare April 6, 2020 03:23
@dcantah dcantah changed the title [WIP] Implement registering for other memory notifications besides oom Implement registering for other memory notifications besides oom Apr 6, 2020
@dcantah

dcantah commented Apr 6, 2020

Copy link
Copy Markdown
Member Author

@crosbymichael PTAL

@crosbymichael

Copy link
Copy Markdown
Member

LGTM

@crosbymichael crosbymichael merged commit 7fc7a50 into containerd:master Apr 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants