rpuch commented on code in PR #2838:
URL: https://github.com/apache/ignite-3/pull/2838#discussion_r1399006102
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/configuration/schema/PersistentPageMemoryDataRegionConfigurationSchema.java:
##########
@@ -28,8 +29,9 @@
*/
@Config
public class PersistentPageMemoryDataRegionConfigurationSchema extends
BasePageMemoryDataRegionConfigurationSchema {
- /** Default size. */
- public static final long DFLT_DATA_REGION_SIZE = 256 * MiB;
+ /** Default size, maximum between 256 MB and 20% of the total physical
memory. */
+ @SuppressWarnings("NumericCastThatLosesPrecision")
+ public static final long DFLT_DATA_REGION_SIZE = Math.max(256 * MiB,
(long) (0.2 * getTotalMemoryAvailable()));
Review Comment:
`getTotalMemoryAvailable()` might return `-1`, but here we are not ready to
this. Let's either handle this (like use 256Mb if the method returns `-1`) or
throw an exception/error from the method instead of returning `-1`. Or maybe
even declare it to return `OptionalLong`/`Optional` to make this specific and
allow the caller handle the missing value.
##########
modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/LowWatermarkConfigurationSchema.java:
##########
@@ -41,7 +41,7 @@ public class LowWatermarkConfigurationSchema {
// TODO https://issues.apache.org/jira/browse/IGNITE-18977 Make these
values configurable and create dynamic validator after that.
@Range(min = DEFAULT_IDLE_SAFE_TIME_PROPAGATION_PERIOD_MILLISECONDS +
CLOCK_SKEW)
@Value(hasDefault = true)
- public long dataAvailabilityTime = TimeUnit.MINUTES.toMillis(45);
+ public long dataAvailabilityTime = TimeUnit.MINUTES.toMillis(10);
Review Comment:
Why is this changed? How were both values (45 and 10) chosen, BTW?
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/mv/RemoveWriteOnGcInvokeClosure.java:
##########
@@ -168,8 +178,10 @@ private RowVersion readRowVersionWithChecks(VersionChain
versionChain) {
void afterCompletion() {
toRemove.forEach(storage::removeRowVersion);
- if (toUpdate != null && !result.hasNextLink()) {
- storage.gcQueue.remove(rowId, toUpdate.timestamp(),
toUpdate.link());
+ if (toDropFromQueue != null) {
+ boolean remove = storage.gcQueue.remove(rowId,
toDropFromQueue.timestamp(), toDropFromQueue.link());
Review Comment:
```suggestion
boolean removed = storage.gcQueue.remove(rowId,
toDropFromQueue.timestamp(), toDropFromQueue.link());
```
--
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]
