keith-turner commented on code in PR #4317:
URL: https://github.com/apache/accumulo/pull/4317#discussion_r1508172702
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java:
##########
@@ -69,7 +72,23 @@ public class TabletManagementIterator extends
SkippingIterator {
private TabletBalancer balancer;
private static boolean shouldReturnDueToSplit(final TabletMetadata tm,
- final long splitThreshold) {
+ final Configuration tableConfig) {
+
+ final long splitThreshold = ConfigurationTypeHelper
+
.getFixedMemoryAsBytes(tableConfig.get(Property.TABLE_SPLIT_THRESHOLD.getKey()));
+ final long maxEndRowSize = ConfigurationTypeHelper
+
.getFixedMemoryAsBytes(tableConfig.get(Property.TABLE_MAX_END_ROW_SIZE.getKey()));
+ final int maxFilesToOpen = (int)
ConfigurationTypeHelper.getFixedMemoryAsBytes(
+
tableConfig.get(Property.TSERV_TABLET_SPLIT_FINDMIDPOINT_MAXOPEN.getKey()));
+
+ // If current config/files match unsplittable metadata then we can't split
+ if (Optional
+ .ofNullable(tm.getUnSplittable()).filter(um ->
um.equals(UnSplittableMetadata
Review Comment:
Was curious if this would allocate an object. Was looking into the impl of
Optional.ofNullable() and found that it does not allocate an object when null
is passed in, it always returns same empty object. So since this will normally
be null it will not allocate anything usually.
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java:
##########
@@ -381,6 +384,11 @@ public boolean getHostingRequested() {
return onDemandHostingRequested;
}
+ public UnSplittableMetadata getUnSplittable() {
Review Comment:
This comment is not related to this line. Need to update the toString()
method on this class.
##########
server/manager/src/main/java/org/apache/accumulo/manager/tableOps/split/FindSplits.java:
##########
@@ -75,13 +88,17 @@ public Repo<Manager> call(FateId fateId, Manager manager)
throws Exception {
if (splits.isEmpty()) {
log.info("Tablet {} needs to split, but no split points could be found.",
tabletMetadata.getExtent());
- // ELASTICITY_TODO record the fact that tablet is un-splittable in
metadata table in a new
- // column. Record the config used to reach this decision and a hash of
the file. The tablet
- // mgmt iterator can inspect this column and only try to split the
tablet when something has
- // changed.
+
+ // TODO: Do we care about conditional mutations here? I don't think it
is required because
Review Comment:
Do want to use conditional mutations for at least two reasons.
* The tablet may no longer exists at this point, so want to avoid writing
junk to the metadata table. Conditional mutation will make sure the tablet
sill exists.
* Do not want to make any changes if an operation id is set on the tablet.
Once a fate operation has set an operation id on a tablet it assumes nothing
else will update it.
Could also require the set of files to be the same as when we read them
using a condtional mutation, but this is not required for correctness like the
comment says.
--
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]
