-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[improve] PIP-381: Handle PositionInfo that's too large to serialize as a single entry #22799
base: master
Are you sure you want to change the base?
Changes from 21 commits
e014356
33e4c71
6d1b93a
568d446
0240250
ed8df4d
c2f0908
27152ff
564a668
0d23d5b
08af8fc
e8d3930
89adf38
43a5b31
8db72f4
1397faf
d4b4195
2fdbe63
9b88801
7918f21
048142c
590c5ac
54157d8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1941,10 +1941,17 @@ managedLedgerMaxUnackedRangesToPersistInZooKeeper=-1 | |
# persist, it will help to reduce the duplicates caused by the ack state that can not be fully persistent. | ||
dispatcherPauseOnAckStatePersistentEnabled=false | ||
|
||
# If enabled, the maximum "acknowledgment holes" will not be limited and "acknowledgment holes" are stored in | ||
# multiple entries. | ||
# If enabled, the maximum "acknowledgment holes" (as defined by managedLedgerMaxUnackedRangesToPersist) | ||
# can be stored in multiple entries, allowing the higher limits. | ||
persistentUnackedRangesWithMultipleEntriesEnabled=false | ||
|
||
# If persistentUnackedRangesWithMultipleEntriesEnabled, this sets maximum entry size for storage in bytes. | ||
persistentUnackedRangesMaxEntrySize=1048576 | ||
|
||
# Set the compression type to use for cursor info. | ||
# Possible options are NONE, LZ4, ZLIB, ZSTD, SNAPPY | ||
cursorInfoCompressionType=NONE | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we already have lot of ServiceConfig and I guess we should define best compression type and use it by default instead giving tuning parameter. introducing 3 tuning config for a simple feature is not required. I guess we should introduce later if we see the need of this config. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's not break backwards compatibility |
||
|
||
# Deprecated - Use managedLedgerCacheEvictionIntervalMs instead | ||
managedLedgerCacheEvictionFrequency=0 | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,7 +45,9 @@ public class ManagedLedgerConfig { | |
private boolean createIfMissing = true; | ||
private int maxUnackedRangesToPersist = 10000; | ||
private int maxBatchDeletedIndexToPersist = 10000; | ||
private String cursorInfoCompressionType = "NONE"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's use the best possible compression algo by performing various tests. performing perf tests for each type for such internal implementation is difficult for any user and it's the author's responsibility to give those numbers. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's not break backwards compatibility There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isnt' it part of this new feature? then how come it will impact backward compatibility? if we will introduce it in this feature then we will have compatibility issue and that's what I would like to avoid by not adding it here, |
||
private boolean persistentUnackedRangesWithMultipleEntriesEnabled = false; | ||
private int persistentUnackedRangesMaxEntrySize = 1024 * 1024; | ||
private boolean deletionAtBatchIndexLevelEnabled = true; | ||
private int maxUnackedRangesToPersistInMetadataStore = 1000; | ||
private int maxEntriesPerLedger = 50000; | ||
|
@@ -480,14 +482,49 @@ public int getMaxBatchDeletedIndexToPersist() { | |
return maxBatchDeletedIndexToPersist; | ||
} | ||
|
||
/** | ||
* @return true if persistent unacked ranges with multiple entries enabled. | ||
*/ | ||
public boolean isPersistentUnackedRangesWithMultipleEntriesEnabled() { | ||
return persistentUnackedRangesWithMultipleEntriesEnabled; | ||
} | ||
|
||
/** | ||
* If enabled, the maximum "acknowledgment holes" will be stored in multiple entries, allowing the higher limits. | ||
* @param multipleEntriesEnabled | ||
*/ | ||
public void setPersistentUnackedRangesWithMultipleEntriesEnabled(boolean multipleEntriesEnabled) { | ||
this.persistentUnackedRangesWithMultipleEntriesEnabled = multipleEntriesEnabled; | ||
} | ||
|
||
/** | ||
* @return max entry size for persistent unacked ranges. | ||
*/ | ||
public int getPersistentUnackedRangesMaxEntrySize() { | ||
return persistentUnackedRangesMaxEntrySize; | ||
} | ||
|
||
/** | ||
* If persistentUnackedRangesWithMultipleEntriesEnabled, this sets maximum entry size for storage in bytes. | ||
*/ | ||
public void setPersistentUnackedRangesMaxEntrySize(int persistentUnackedRangesMaxEntrySize) { | ||
this.persistentUnackedRangesMaxEntrySize = persistentUnackedRangesMaxEntrySize; | ||
} | ||
|
||
/** | ||
* @return compression type to use for cursor info. | ||
*/ | ||
public String getCursorInfoCompressionType() { | ||
return cursorInfoCompressionType; | ||
} | ||
|
||
/** | ||
* Set the compression type to use for cursor info. | ||
*/ | ||
public void setCursorInfoCompressionType(String cursorInfoCompressionType) { | ||
this.cursorInfoCompressionType = cursorInfoCompressionType; | ||
} | ||
|
||
/** | ||
* @param maxUnackedRangesToPersist | ||
* max unacked message ranges that will be persisted and receverd. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should define that higher limit because again it will confuse user and no one will have any idea about this limit. PR #9292 by default supports up to 10M unack messages in single entry,. So, we should define higher limit as
> 10M unack messages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not break backwards compatibility given that feature is disabled by default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how does it break the compatibility by just updating the comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also this is a new feature to write in multiple entry from where backward compatibility came from :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It writes to the same ledger as existing single entry write.