[GitHub] [pulsar] sijie commented on a change in pull request #8618: [PIP 70][Issue 8617] Introduce lightweight raw Message metadata

[GitBox <gi...@apache.org>]

sijie commented on a change in pull request #8618:
URL: https://github.com/apache/pulsar/pull/8618#discussion_r539075703



##########
File path: managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedgerConfig.java
##########
@@ -75,6 +75,7 @@
     private LedgerOffloader ledgerOffloader = NullLedgerOffloader.INSTANCE;
     private int newEntriesCheckDelayInMillis = 10;
     private Clock clock = Clock.systemUTC();
+    private boolean brokerTimestampForMessageEnable = false;

Review comment:
       broker timestamp is a concept on pulsar broker. managed ledger isn't aware of any broker features. I am not sure why we need this flag here or the name of this configuration setting is not appropriate. 

##########
File path: pulsar-common/src/main/proto/PulsarApi.proto
##########
@@ -180,6 +180,11 @@ message SingleMessageMetadata {
     optional bool null_partition_key = 10 [ default = false];
 }
 
+// raw metadata for message
+message RawMessageMetadata {

Review comment:
       I think the original idea to introduce RawMessageMetadata is to avoid serialization and deserialization. So broker doesn't need to serialize another protobuf message and create a new entry to store in bookkeeper side. I am not sure why do we need this.

##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -1428,6 +1428,11 @@
                     + "Of course, this may degrade consumption throughput. Default is 10ms.")
     private int managedLedgerNewEntriesCheckDelayInMillis = 10;
 
+    @FieldContext(category = CATEGORY_STORAGE_ML,
+            doc = "Enable broker side timestamp for message. Default is false.")
+    private boolean brokerTimestampForMessageEnable = false;

Review comment:
       ```suggestion
       private boolean brokerTimestampForMessageEnabled = false;
   ```




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org