Skip to content

Conversation

@congbobo184
Copy link
Contributor

Motivation

to fix filesystem offload oom #6692

Does this pull request potentially affect one of the following parts:
If yes was chosen, please highlight the changes

Dependencies (does it add or upgrade a dependency): (no)
The public API: (no)
The schema: (no)
The default values of configurations: (no)
The wire protocol: (no)
The rest endpoints: (no)
The admin cli options: (no)
Anything that affects deployment: (no)

Documentation

Does this pull request introduce a new feature? (yes / no)
If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
If a feature is not applicable for documentation, explain why?
If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

@codelipenghui codelipenghui added the type/bug The PR fixed a bug or issue reported a bug label Apr 9, 2020
@codelipenghui codelipenghui added this to the 2.6.0 milestone Apr 9, 2020
@codelipenghui codelipenghui linked an issue Apr 9, 2020 that may be closed by this pull request
@congbobo184
Copy link
Contributor Author

/pulsarbot run-failure-checks

@sijie
Copy link
Member

sijie commented Apr 9, 2020

/pulsarbot run-failure-checks

2 similar comments
@congbobo184
Copy link
Contributor Author

/pulsarbot run-failure-checks

@congbobo184
Copy link
Contributor Author

/pulsarbot run-failure-checks

@pheecian
Copy link
Contributor

@congbobo184 thanks for your pull request. I have applied the modification. However, the OOM still occurs.
Screen Shot 2020-04-10 at 3 10 44 PM

@congbobo184
Copy link
Contributor Author

congbobo184 commented Apr 11, 2020

@congbobo184 thanks for your pull request. I have applied the modification. However, the OOM still occurs.
Screen Shot 2020-04-10 at 3 10 44 PM

I think you offload to hdfs is too slowly, and then you produce message is too fast. and then many ledger will offload int the same time. so we maybe should control the read-ahead number or the whole fileSystem offload task number

Copy link
Contributor

@pheecian pheecian left a comment

Choose a reason for hiding this comment

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

close ledgerEntries at once or close ledger entry one by one is equivalent. this will make no difference for the memory consumption. the same idea(actually the same code modification) is tested by me already. It will not help. Anyway, it makes sense. But it helps not.

@pheecian
Copy link
Contributor

quote from @congbobo184 "so we maybe should control the read-ahead number" this is what my PR want to do

@tuteng
Copy link
Member

tuteng commented Apr 13, 2020

Add label release/2.5.1 and remove label release/2.5.2

codelipenghui pushed a commit that referenced this pull request Apr 22, 2020
Fixes #6692 

### Motivation
avoid prefetch too much data when offloading, which may lead to OOM;
fix object not close issue, which is also mentioned by congbobo184 #6697

*Explain here the context, and why you're making that change. What is the problem you're trying to solve.*

### Does this pull request potentially affect one of the following parts:

*If `yes` was chosen, please highlight the changes*

  - Dependencies (does it add or upgrade a dependency): (no)
  - The public API: (no)
  - The schema: (no)
  - The default values of configurations: (no)
  - The wire protocol: (no)
  - The rest endpoints: (no)
  - The admin cli options: (no)
  - Anything that affects deployment: (no)

### Documentation

  - Does this pull request introduce a new feature? (no)
addisonj pushed a commit to instructure/pulsar that referenced this pull request May 7, 2020
Fixes apache#6692

avoid prefetch too much data when offloading, which may lead to OOM;
fix object not close issue, which is also mentioned by congbobo184 apache#6697

*Explain here the context, and why you're making that change. What is the problem you're trying to solve.*

*If `yes` was chosen, please highlight the changes*

  - Dependencies (does it add or upgrade a dependency): (no)
  - The public API: (no)
  - The schema: (no)
  - The default values of configurations: (no)
  - The wire protocol: (no)
  - The rest endpoints: (no)
  - The admin cli options: (no)
  - Anything that affects deployment: (no)

  - Does this pull request introduce a new feature? (no)
jiazhai pushed a commit that referenced this pull request May 8, 2020
Fixes #6692

### Motivation
avoid prefetch too much data when offloading, which may lead to OOM;
fix object not close issue, which is also mentioned by congbobo184 #6697

*Explain here the context, and why you're making that change. What is the problem you're trying to solve.*

### Does this pull request potentially affect one of the following parts:

*If `yes` was chosen, please highlight the changes*

  - Dependencies (does it add or upgrade a dependency): (no)
  - The public API: (no)
  - The schema: (no)
  - The default values of configurations: (no)
  - The wire protocol: (no)
  - The rest endpoints: (no)
  - The admin cli options: (no)
  - Anything that affects deployment: (no)

### Documentation

  - Does this pull request introduce a new feature? (no)
(cherry picked from commit 514b6af)
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Aug 24, 2020
Fixes apache#6692 

### Motivation
avoid prefetch too much data when offloading, which may lead to OOM;
fix object not close issue, which is also mentioned by congbobo184 apache#6697

*Explain here the context, and why you're making that change. What is the problem you're trying to solve.*

### Does this pull request potentially affect one of the following parts:

*If `yes` was chosen, please highlight the changes*

  - Dependencies (does it add or upgrade a dependency): (no)
  - The public API: (no)
  - The schema: (no)
  - The default values of configurations: (no)
  - The wire protocol: (no)
  - The rest endpoints: (no)
  - The admin cli options: (no)
  - Anything that affects deployment: (no)

### Documentation

  - Does this pull request introduce a new feature? (no)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release/2.5.1 type/bug The PR fixed a bug or issue reported a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[v2.5.0]memory leak during offload to tiered storage

6 participants