Skip to content

[fix][managed-ledger]persistentMarkDeletePosition is ahead of markDeletePosition in ManagedCursor when handling seek request #6

Open
HQebupt wants to merge 1137 commits intomasterfrom
seekMarkdeleteBug
Open

[fix][managed-ledger]persistentMarkDeletePosition is ahead of markDeletePosition in ManagedCursor when handling seek request #6
HQebupt wants to merge 1137 commits intomasterfrom
seekMarkdeleteBug

Conversation

@HQebupt
Copy link
Owner

@HQebupt HQebupt commented Oct 19, 2022

Motivation

When the client seek a position, it will update the read posistion and mark delete position in the cursor. And meanwhile, it will persist the mark delete position to Bookie, then record the persistent mark delete position in the field persistentMarkDeletePosition.

However, The persistent mark delete position persistentMarkDeletePosition is ahead of markDeletePosition after seek , which is obviously unreasonable.

https://github.com/apache/pulsar/blob/cbbcd41cfc80793cf025ef98390339395ecd48ac/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java#L125-L132

Modifications

  • Persistent the previous seek position as new persistent mark delete position into Bookie.
  • Clean up code

Verifying this change

  • Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

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

Check the box below and label this PR (if you have committer privilege).

Need to update docs?

  • doc-not-needed

Matching PR in forked repository

PR in forked repository: #6

BewareMyPower and others added 30 commits September 16, 2022 10:32
* Create 2022-09-09-Apache-Pulsar-2-7-5.md

* Update site2/website/blog/2022-09-09-Apache-Pulsar-2-7-5.md

Co-authored-by: Anonymitaet <50226895+Anonymitaet@users.noreply.github.com>

Co-authored-by: Anonymitaet <50226895+Anonymitaet@users.noreply.github.com>
* [improve][doc] add developers-landing page

The Pulsar for Developers content block on the documentation landing page needs to link to this page.

* Update getting-started-standalone.md

* Update getting-started-standalone.md

* move the file to /site2/docs/

* Update about.md

* Update site2/docs/developers-landing.md

Co-authored-by: tison <wander4096@gmail.com>

* Update site2/docs/getting-started-standalone.md

Co-authored-by: tison <wander4096@gmail.com>

Co-authored-by: momo-jun <jma@streamnative.io>
Co-authored-by: tison <wander4096@gmail.com>
…th a relative path (apache#17675)

* [improve][cli] Pulsar shell: allow to create a new config (--file) with a relative path

* win

* checkstlye
…pache#17687)

* Fix parsing partitionedKey with Base64 encode issue.

* release the buf

* fix checkstyle issue.
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
…tible across different versions (apache#17690)

* [fix][functions] Ensure InternalConfigurationData data model is compatible across different versions

* style

* fix the other way

### Motivation

After apache#14384, the broker and the client expects that the `InternalConfigurationData` contains `metadataStoreUrl` and `configurationMetadataStoreUrl` fields.
However the broker is no more compatible with old clients. 


apache#14384 is landed to branch-2.11 and [2.10.2](apache#17291)

Example scenario: 
- broker on 2.10.1
- function worker on 2.10.1

1. upgrade fn worker to 2.11.0 or 2.10.2
2. the fn worker starts and download the internal config from the broker
3. broker serves a json with old fields (`zookeeperServers` and `configurationStoreServers`)
4. fn worker reads the json and convert it to a `InternalConfigurationData` instance. It expects to see the fields filled `metadataStoreUrl` and `configurationMetadataStoreUrl` but they aren't
5. NPE on fn worker 
```
2022-09-15T17:42:16,072+0000 [main] INFO  org.apache.pulsar.functions.worker.PulsarWorkerService - Initializing Pulsar Functions namespace...
2022-09-15T17:42:16,192+0000 [main] ERROR org.apache.pulsar.functions.worker.FunctionWorkerStarter - Encountered error in function worker.
java.lang.NullPointerException: null
    at org.apache.pulsar.metadata.impl.MetadataStoreFactoryImpl.removeIdentifierFromMetadataURL(MetadataStoreFactoryImpl.java:73) 
    at org.apache.pulsar.functions.worker.WorkerUtils.initializeDlogNamespace(WorkerUtils.java:188) 
    at org.apache.pulsar.functions.worker.PulsarWorkerService.initializeStandaloneWorkerService(PulsarWorkerService.java:281) 
    at org.apache.pulsar.functions.worker.PulsarWorkerService.initAsStandalone(PulsarWorkerService.java:208)
    at org.apache.pulsar.functions.worker.Worker.start(Worker.java:54)
    at org.apache.pulsar.functions.worker.FunctionWorkerStarter.main(FunctionWorkerStarter.java:76)
```

Additionaly there's the same issue if we upgrade the broker before the fn worker:
1. the broker gets the upgrade. won't serve `zookeeperServers` field
2. fn worker restarts for some reasons. 
3. fn worker gets the internal config and look for  `zookeeperServers` field which is empty in the json
4. NPE 

### Modifications

* Restore old fields in `InternalConfigurationData` and add fallback the old values in the new fields getters
* Added unit test 

- [x] `doc-not-needed`
…nsumption (apache#17693)

Please check the proposal email on dev mailing list: https://lists.apache.org/thread/gwfmxmxlhtsjn17sxxc367jcs4pcwofz

Co-authored-by: tison <wander4096@gmail.com>
… used to terminate the process (apache#17589)

* [fix][common] Fix issue where logs get truncated when Pulsar process gets terminated with Runtime.halt

- call Log4j2's LogManager.shutdown before terminating the process with Runtime.halt

* Don't log warning when status code is 0
Signed-off-by: tison <wander4096@gmail.com>

Signed-off-by: tison <wander4096@gmail.com>
…e#17723)

- The approval solution doesn't work as expected by approving the PR
  or by adding the ready-to-test label and adding a comment
  "/pulsarbot rerun-failure-checks".

- Fix the ready-to-test label check:
  - Refresh PR labels when re-running workflow
    - when re-running, the event JSON remains the same. The API
      must be used to fetch the up-to-date JSON for the PR.

- Fix the PR approval check:
  - set GITHUB_TOKEN for script so that retrieving the approval status could work
### Motivation

in the schema update, will create a `ledgerHandle` and write data to BK, after that `ledgerHandle` is no longer useful and no other object holds references to it. `ledgerHandle` will be recycled with GC, but `ledgerHandle` also hold external connections, which will cause leakage.

https://github.com/apache/pulsar/blob/40b9d7ea50cef54becb09f2543193e08375abe0b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/schema/BookkeeperSchemaStorage.java#L452-L456

### Modifications

after the schema is updated, close the `ledgerHandle`, just like schema-read:

https://github.com/apache/pulsar/blob/40b9d7ea50cef54becb09f2543193e08375abe0b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/schema/BookkeeperSchemaStorage.java#L519-L525
…hapter (apache#17615)

* Sidebar re-org

* Separate TLS encryption/authentication using Keystore to parent topics

* use language-specific tabs to show code snippets

* streamline headings

* Update client-libraries-java.md

* Add code snippet for python clients to support apache#17482

* Add code snippet for go clients
coderzc and others added 26 commits October 17, 2022 11:45
…22-40149 (apache#18022)

* [fix][sec] File tiered storage: upgrade jettison to get rid of CVE-2022-40149

* fix
…ntryCacheBehavior (apache#18075)

Co-authored-by: leolinchen <leolinchen@tencent.com>
…TypeDefinitions (apache#18077)

* [fix][test] Fix flaky test: PrometheusMetricsTest.testDuplicateMetricTypeDefinitions

* fix
…che#18008)

* [improve][io] JDBC Sink: add flag to exclude non declared fields

* rename and doc

* fix doc

* Update pulsar-io/jdbc/core/src/main/java/org/apache/pulsar/io/jdbc/JdbcSinkConfig.java

Co-authored-by: tison <wander4096@gmail.com>

Co-authored-by: tison <wander4096@gmail.com>
* [fix][ci] Fix deprecation warnings about set-output

Upstream fix: amannn/action-semantic-pull-request#208

* Update .github/workflows/ci-semantic-pull-request.yml
…ad position for consumer (apache#18037)

### Motivation
Fixes apache#18036

### Modifications
- The backlog eviction policy should use `asyncMarkDelete` instead of `resetCursor` in order to move the mark delete position.
…etePosition in ManagedCursor when handle seek request
@github-actions
Copy link

The pr had no activity for 30 days, mark with Stale label.

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 this pull request may close these issues.