-
Notifications
You must be signed in to change notification settings - Fork 444
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
Add Mergeability column to support automatic merges #5187
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
---|---|---|
@@ -0,0 +1,86 @@ | ||
/* | ||
* Licensed to the Apache Software Foundation (ASF) under one | ||
* or more contributor license agreements. See the NOTICE file | ||
* distributed with this work for additional information | ||
* regarding copyright ownership. The ASF licenses this file | ||
* to you under the Apache License, Version 2.0 (the | ||
* "License"); you may not use this file except in compliance | ||
* with the License. You may obtain a copy of the License at | ||
* | ||
* https://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
package org.apache.accumulo.core.client.admin; | ||
|
||
import java.io.Serializable; | ||
import java.time.Duration; | ||
import java.util.Objects; | ||
|
||
import com.google.common.base.Preconditions; | ||
|
||
public class TabletMergeability implements Serializable { | ||
private static final long serialVersionUID = 1L; | ||
|
||
public static final TabletMergeability NEVER = new TabletMergeability(Duration.ofNanos(-1)); | ||
public static final TabletMergeability NOW = new TabletMergeability(Duration.ofNanos(0)); | ||
|
||
private final Duration delay; | ||
|
||
private TabletMergeability(Duration delay) { | ||
this.delay = Objects.requireNonNull(delay); | ||
} | ||
|
||
public boolean isNever() { | ||
return this.delay.isNegative(); | ||
} | ||
|
||
public boolean isNow() { | ||
return this.delay.isZero(); | ||
} | ||
|
||
public Duration getDelay() { | ||
return delay; | ||
} | ||
|
||
@Override | ||
public boolean equals(Object o) { | ||
if (o == null || getClass() != o.getClass()) { | ||
return false; | ||
} | ||
TabletMergeability that = (TabletMergeability) o; | ||
return Objects.equals(delay, that.delay); | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hashCode(delay); | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
if (isNow()) { | ||
return "TabletMergeability=NOW"; | ||
} else if (isNever()) { | ||
return "TabletMergeability=NEVER"; | ||
} | ||
return "TabletMergeability=AFTER:" + delay.toNanos(); | ||
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 may want to convert this to 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. Yeah, not a bad idea, nanoseconds would be pretty tough to understand in a log. |
||
} | ||
|
||
public static TabletMergeability from(Duration delay) { | ||
Preconditions.checkArgument(delay.toNanos() >= -1, | ||
"Duration of delay must be -1, 0, or a positive offset."); | ||
return new TabletMergeability(delay); | ||
} | ||
|
||
public static TabletMergeability after(Duration delay) { | ||
Preconditions.checkArgument(delay.toNanos() > 0, "Duration of delay must be greater than 0."); | ||
return new TabletMergeability(delay); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
/* | ||
* Licensed to the Apache Software Foundation (ASF) under one | ||
* or more contributor license agreements. See the NOTICE file | ||
* distributed with this work for additional information | ||
* regarding copyright ownership. The ASF licenses this file | ||
* to you under the Apache License, Version 2.0 (the | ||
* "License"); you may not use this file except in compliance | ||
* with the License. You may obtain a copy of the License at | ||
* | ||
* https://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
package org.apache.accumulo.core.metadata; | ||
|
||
import java.time.Duration; | ||
|
||
import org.apache.accumulo.core.client.admin.TabletMergeability; | ||
import org.apache.accumulo.core.data.Value; | ||
import org.apache.accumulo.core.util.time.SteadyTime; | ||
|
||
public class TabletMergeabilityUtil { | ||
|
||
public static Value toValue(TabletMergeability tabletMergeability) { | ||
return new Value(Long.toString(tabletMergeability.getDelay().toNanos())); | ||
} | ||
|
||
public static TabletMergeability fromValue(Value value) { | ||
return TabletMergeability.from(Duration.ofNanos(Long.parseLong(value.toString()))); | ||
} | ||
|
||
public boolean isMergeable(TabletMergeability mergeability, SteadyTime currentTime) { | ||
if (mergeability.isNever()) { | ||
return false; | ||
} | ||
return currentTime.getDuration().compareTo(mergeability.getDelay()) >= 0; | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -27,6 +27,7 @@ | |||||
|
||||||
import org.apache.accumulo.core.client.ConditionalWriter; | ||||||
import org.apache.accumulo.core.client.admin.TabletAvailability; | ||||||
import org.apache.accumulo.core.client.admin.TabletMergeability; | ||||||
import org.apache.accumulo.core.data.Key; | ||||||
import org.apache.accumulo.core.data.Mutation; | ||||||
import org.apache.accumulo.core.data.TableId; | ||||||
|
@@ -392,6 +393,8 @@ interface TabletUpdates<T> { | |||||
|
||||||
T putCloned(); | ||||||
|
||||||
T putTabletMergeability(TabletMergeability tabletMergeability); | ||||||
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. Eventually we will need a steady time to evaluate if something should merge. Seems like the steady time should always be set when the mergability is set. Although its only needed when the duration is > 0. The steady time could be encoded in the same columns value.
Suggested change
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. Steady time could be added in later PRs. Just wondering how it will be persisted. 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. My original plan was to persist everything as one value by adding the current manager time to the value. So basically to compute the mergability duration you would read the current SteadyTime then add the offset to it and store it and then later you could compare to the current time by taking a diff. However, I was thinking more about it and I think that storing it as two separate values makes sense because it allows you to do better debugging (logigng) plus you can see extra information such as when the value was created. It also allows doing other things like update/replacing the original SteadyTime, etc and other metric calculations if we keep it separate so I'll change it. 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.
Ok I had thought this would store the exact duration specified by the user. Summing the steady time and duration from the user would work. There may be a slight advantage for keeping them separate for debugging purposes as mentioned. |
||||||
|
||||||
/** | ||||||
* By default the server lock is automatically added to mutations unless this method is set to | ||||||
* false. | ||||||
|
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.