codelipenghui commented on issue #16763: URL: https://github.com/apache/pulsar/issues/16763#issuecomment-1219606491
> Delete message index bucket After merging buckets, the delayed message tracker will delete the old bucket. Also, when all the delayed messages of all snapshots in the bucket are scheduled then tracker will delete that bucket. We need to explain how snapshot can be guaranteed to be deleted without being missed. > Admin-API changes for clear snapshot Could you please add more context about why we need this API? to rebuild the index? It should be a risk if only clear the index without any cursor rewind or index rebuilding. The consumer will not be able to receive delayed messages, no? For the metrics we are using `pulsar_delayed_message_index_*` https://github.com/apache/pulsar/pull/15867/files#diff-b3f64ff76fdabb9e471435147298c3c707cbecae09023f0d86cf06f6ad78cbdaR352. It's better to keep the metrics name consistent. ``` pulsar_delayed_message_index_bucket_total | Gauge pulsar_delayed_message_index_snapshot_size_bytes | Gauge pulsar_delayed_message_index_bucket_op_latency_ms {type="load/merge/del"} | Histogram pulsar_delayed_message_index_bucket_op_failed_count | Counter ``` For the BucketSnapshotFormat.proto Is it better to split SnapshotMetadata and SnapshotData into 2 commands? It's easier to read. The metadata message have bitset and scheduleTime, the data message only has the sorted list. And we'd better change `BucketSnapshotFormat.proto` to `DelayedMessageIndexBucketSnapshotFormat.proto` > CompletableFuture<Void> saveBucketAllSnapshots(long bucketId, List<BucketSnapshotFormat.BucketSnapshot> bucketSnapshots); It's a little confusing for this one. For each bucket, we should only have one snapshot, for each entry in the snapshot, we should use a different name, snapshot record or something. > CompletableFuture<Long> createBucket(); I think this one is used to create a snapshot? > CompletableFuture<Void> deleteBucket(long bucketId); is this one used to delete a snapshot? > void start() throws Exception; What does this method really do? Does it look like `initialize()`? It is hard to comment on the issue, it looks like we need to find a new approach to create and review the proposal. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
