codelipenghui commented on code in PR #18801:
URL: https://github.com/apache/pulsar/pull/18801#discussion_r1043951351
##########
pulsar-metadata/src/test/java/org/apache/pulsar/metadata/bookkeeper/PulsarLedgerIdGeneratorTest.java:
##########
@@ -49,8 +49,8 @@ public class PulsarLedgerIdGeneratorTest extends
BaseMetadataStoreTest {
@Test(dataProvider = "impl")
public void testGenerateLedgerId(String provider, Supplier<String>
urlSupplier) throws Exception {
@Cleanup
- MetadataStoreExtended store =
- MetadataStoreExtended.create(urlSupplier.get(),
MetadataStoreConfig.builder().build());
+ MetadataStoreExtended store =
MetadataStoreExtended.create(urlSupplier.get(),
+
MetadataStoreConfig.builder().fsyncEnable(false).build());
Review Comment:
Please also help check other tests that use the fsync mode.
##########
conf/standalone.conf:
##########
@@ -29,6 +29,11 @@ metadataStoreUrl=
# Configuration file path for metadata store. It's supported by
RocksdbMetadataStore and EtcdMetadataStore for now
metadataStoreConfigPath=
+# Whether we should enable fsync for local metadata store. It's supported by
RocksdbMetadataStore for now
+# If this flag is true, metadata writes will be slower.
+# If this flag is false, and the machine crashes, some recent metadata writes
may be lost.
+# Note that if it is just the process that crashes (i.e., the machine does not
reboot), no writes will be lost even if it is false.
+metadataFsyncEnabled=true
Review Comment:
Maybe we don't need to expose it to users?
We just want to improve the test.
Suppose we allow users to configure with false. The machine crash might lead
to the metadata being corrupted.
##########
conf/standalone.conf:
##########
@@ -29,6 +29,11 @@ metadataStoreUrl=
# Configuration file path for metadata store. It's supported by
RocksdbMetadataStore and EtcdMetadataStore for now
metadataStoreConfigPath=
+# Whether we should enable fsync for local metadata store. It's supported by
RocksdbMetadataStore for now
+# If this flag is true, metadata writes will be slower.
+# If this flag is false, and the machine crashes, some recent metadata writes
may be lost.
+# Note that if it is just the process that crashes (i.e., the machine does not
reboot), no writes will be lost even if it is false.
+metadataFsyncEnabled=true
Review Comment:
Or we can expose it until we need it. I haven't seen any issues or
discussions about this part.
And batch operations might be another choice to improve the rocksdb metadata
performance.
--
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]
