You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2022/09/27 14:20:30 UTC
[GitHub] [accumulo] milleruntime opened a new pull request, #2969: Add missing chopped column to Ample
milleruntime opened a new pull request, #2969:
URL: https://github.com/apache/accumulo/pull/2969
* Fixes #2961
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] keith-turner commented on a diff in pull request #2969: Add missing chopped column to Ample
Posted by GitBox <gi...@apache.org>.
keith-turner commented on code in PR #2969:
URL: https://github.com/apache/accumulo/pull/2969#discussion_r981378403
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java:
##########
@@ -399,6 +401,9 @@ public static <E extends Entry<Key,Value>> TabletMetadata convertRow(Iterator<E>
extCompBuilder.put(ExternalCompactionId.of(qual),
ExternalCompactionMetadata.fromJson(val));
break;
+ case ChoppedColumnFamily.STR_NAME:
+ te.chopped = true;
Review Comment:
Could do this if don't want to add a method to get chopped.
```suggestion
```
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java:
##########
@@ -106,6 +107,7 @@ public class TabletMetadata {
private OptionalLong compact = OptionalLong.empty();
private Double splitRatio = null;
private Map<ExternalCompactionId,ExternalCompactionMetadata> extCompactions;
+ public boolean chopped = false;
Review Comment:
Would be better to make this private and have a method to access it, or could not have it at all. The reason a method would be good is that `ensureFetched()` could be called in the method to make sure it was actually retrieved. All other fields do this. In avoids bugs where someone did not fetch a field from the metadata table but they try to use it. Like if someone read this chopped field but never fetched it, then it would always be false.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on a diff in pull request #2969: Add missing chopped column to Ample
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2969:
URL: https://github.com/apache/accumulo/pull/2969#discussion_r981464788
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java:
##########
@@ -399,6 +401,9 @@ public static <E extends Entry<Key,Value>> TabletMetadata convertRow(Iterator<E>
extCompBuilder.put(ExternalCompactionId.of(qual),
ExternalCompactionMetadata.fromJson(val));
break;
+ case ChoppedColumnFamily.STR_NAME:
+ te.chopped = true;
Review Comment:
I don't follow your suggestion. It looks like you are just deleting the line?
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on a diff in pull request #2969: Add missing chopped column to Ample
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2969:
URL: https://github.com/apache/accumulo/pull/2969#discussion_r981528258
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java:
##########
@@ -399,6 +401,9 @@ public static <E extends Entry<Key,Value>> TabletMetadata convertRow(Iterator<E>
extCompBuilder.put(ExternalCompactionId.of(qual),
ExternalCompactionMetadata.fromJson(val));
break;
+ case ChoppedColumnFamily.STR_NAME:
+ te.chopped = true;
Review Comment:
I am guessing you are saying to have a getter and setter, so we can check the fetched list.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on a diff in pull request #2969: Add missing chopped column to Ample
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2969:
URL: https://github.com/apache/accumulo/pull/2969#discussion_r981571942
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java:
##########
@@ -106,6 +107,7 @@ public class TabletMetadata {
private OptionalLong compact = OptionalLong.empty();
private Double splitRatio = null;
private Map<ExternalCompactionId,ExternalCompactionMetadata> extCompactions;
+ public boolean chopped = false;
Review Comment:
Added a getter and call to ensureChecked in [c2e5554](https://github.com/apache/accumulo/pull/2969/commits/c2e555499e06daa220dd5982a6a021f599dbdfe8)
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime merged pull request #2969: Add missing chopped column to Ample
Posted by GitBox <gi...@apache.org>.
milleruntime merged PR #2969:
URL: https://github.com/apache/accumulo/pull/2969
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] keith-turner commented on a diff in pull request #2969: Add missing chopped column to Ample
Posted by GitBox <gi...@apache.org>.
keith-turner commented on code in PR #2969:
URL: https://github.com/apache/accumulo/pull/2969#discussion_r981575489
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java:
##########
@@ -399,6 +401,9 @@ public static <E extends Entry<Key,Value>> TabletMetadata convertRow(Iterator<E>
extCompBuilder.put(ExternalCompactionId.of(qual),
ExternalCompactionMetadata.fromJson(val));
break;
+ case ChoppedColumnFamily.STR_NAME:
+ te.chopped = true;
Review Comment:
I Was just thinking that just doing nothing when seeing chopped would be ok if the getter is not added now. Nothing would be calling the getter.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] keith-turner commented on a diff in pull request #2969: Add missing chopped column to Ample
Posted by GitBox <gi...@apache.org>.
keith-turner commented on code in PR #2969:
URL: https://github.com/apache/accumulo/pull/2969#discussion_r981381188
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java:
##########
@@ -106,6 +107,7 @@ public class TabletMetadata {
private OptionalLong compact = OptionalLong.empty();
private Double splitRatio = null;
private Map<ExternalCompactionId,ExternalCompactionMetadata> extCompactions;
+ public boolean chopped = false;
Review Comment:
Would be better to make this private and have a method to access it, or could not have it at all. The reason a method would be good is that `ensureFetched()` could be called in the method to make sure it was actually retrieved. All other fields do this. It avoids bugs where someone did not fetch a field from the metadata table but they try to use it. Like if someone read this chopped field but never fetched it, then it would always be false.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org