You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "massakam (via GitHub)" <gi...@apache.org> on 2023/07/31 09:22:32 UTC
[GitHub] [pulsar] massakam opened a new pull request, #20908: [fix][broker] Fix ack hole in cursor for geo-replication
massakam opened a new pull request, #20908:
URL: https://github.com/apache/pulsar/pull/20908
### Motivation
Occasionally there is an ack hole in the cursor for geo-replication. The following is the internal stats for the topic where the problem occurred:
```json
{
"entriesAddedCounter" : 11000,
"numberOfEntries" : 6999,
"totalSize" : 362285,
"currentLedgerEntries" : 6999,
"currentLedgerSize" : 362285,
"lastLedgerCreatedTimestamp" : "2023-07-30T16:17:10.137+09:00",
"waitingCursorsCount" : 1,
"pendingAddEntriesCount" : 0,
"lastConfirmedEntry" : "1687807:6998",
"state" : "LedgerOpened",
"ledgers" : [ {
"ledgerId" : 1687807,
"entries" : 0,
"size" : 0,
"offloaded" : false,
"underReplicated" : false
} ],
"cursors" : {
"pulsar.repl.cluster-a" : {
"markDeletePosition" : "1687807:6002",
"readPosition" : "1687807:6999",
"waitingReadOp" : true,
"pendingReadOps" : 0,
"messagesConsumedCounter" : 10942,
"cursorLedger" : 1687808,
"cursorLedgerLastEntry" : 13,
"individuallyDeletedMessages" : "[(1687807:6060..1687807:6998]]",
"lastLedgerSwitchTimestamp" : "2023-07-30T16:17:10.143+09:00",
"state" : "Open",
"numberOfEntriesSinceFirstNotAckedMessage" : 997,
"totalNonContiguousDeletedMessagesRange" : 1,
"properties" : { }
}
},
"schemaLedgers" : [ ],
"compactedLedger" : {
"ledgerId" : -1,
"entries" : -1,
"size" : -1,
"offloaded" : false,
"underReplicated" : false
}
}
```
This problem occurred in the following situations:
- There are two replication clusters, cluster-a and cluster-b
- cluster-a usually has no producers or consumers, but once a day a producer connects and publishes messages
- Only consumers are connected to cluster-b
- Retention time is 0
In the above case, the producer for geo-replication on the cluster-a side will be closed after a certain period of time by GC.
https://github.com/apache/pulsar/blob/ca01447fb4df808f8e2da2dc75d44fad0b780032/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java#L2626
However, at this time, an already triggered operation to read new entries will not be cancelled. This operation will remain pending until new entries are available.
Then 24 hours later the user's producer connects again and publishes messages. This triggers the pending operation and causes the replicator to start reading new entries.
However, since the producer for geo-replication has not yet been restarted, these read entries will be dropped without being acknowledged.
https://github.com/apache/pulsar/blob/ca01447fb4df808f8e2da2dc75d44fad0b780032/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/GeoPersistentReplicator.java#L138-L149
On the other hand, since the user's producer is connected, the producer for geo-replication is also restarted and the cursor is rewound. After that, the state of the replicator is changed to `Started`.
https://github.com/apache/pulsar/blob/ca01447fb4df808f8e2da2dc75d44fad0b780032/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentReplicator.java#L138-L145
At this time, one of the operations triggered before the cursor is rewound succeeds, causing `readPosition` to move to "the position next to the successfully read entry". Entries before this position will not be read again. As a result, entries that have been read once but not acknowledged will be left as an ack hole.
In short, a race condition between "restarting a producer for georeplication" and "an read operation that was triggered the last time geo-replication occurred" is what causes this issue.
### Modifications
Cancel the pending read request in the method `disableReplicatorRead` which is executed after the geo-replication producer is closed by GC.
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `doc` <!-- Your PR contains doc changes. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] massakam closed pull request #20908: [fix][broker] Fix ack hole in cursor for geo-replication
Posted by "massakam (via GitHub)" <gi...@apache.org>.
massakam closed pull request #20908: [fix][broker] Fix ack hole in cursor for geo-replication
URL: https://github.com/apache/pulsar/pull/20908
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] massakam commented on pull request #20908: [fix][broker] Fix ack hole in cursor for geo-replication
Posted by "massakam (via GitHub)" <gi...@apache.org>.
massakam commented on PR #20908:
URL: https://github.com/apache/pulsar/pull/20908#issuecomment-1658581842
This fix seems to have problems, so I'll think of another way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org