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/08/02 14:17:00 UTC
[GitHub] [accumulo] dlmarion opened a new issue, #2841: Change ZooStore get/set property methods to use enum instead of String for property name
dlmarion opened a new issue, #2841:
URL: https://github.com/apache/accumulo/issues/2841
Changes in #2829 caused a problem in FateConcurrencyIT which might not have happened if the ZooStore get/set property method used an Enum instead of a String for the property name. Investigate whether using an enum for the property name is feasible, and if so, make the change as it should negate issues in the future with property name changes.
--
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.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] cshannon commented on issue #2841: Change ZooStore get/set property methods to use enum instead of String for property name
Posted by GitBox <gi...@apache.org>.
cshannon commented on issue #2841:
URL: https://github.com/apache/accumulo/issues/2841#issuecomment-1203195781
> See[ #2829 ](https://github.com/apache/accumulo/pull/2829) for a discussion on changing the name of prop_debug to something that reflects what is stored, vs how it can be used.
Is the intention here to still try and rename now or just do enums? Renaming something like `prop_debug` to something else would break backwards compatibility for existing data already stored under that node name.
--
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] EdColeman commented on issue #2841: Change ZooStore get/set property methods to use enum instead of String for property name
Posted by GitBox <gi...@apache.org>.
EdColeman commented on issue #2841:
URL: https://github.com/apache/accumulo/issues/2841#issuecomment-1203245959
To clarify why I think the rename is important to improve code readability and being able to understand FATE transactions.
The name prop_debug and its usage does not convey what the information is. To unwind and understand the FATE structure is not straight forward - the documentation has been updated to help some, but that does not make the code easier to follow.
Having a name that reflects its usage would help admins that may be poking around in ZooKeeper troubleshooting as well as future development. Each time this code is revisited, requiring someone to either know what prop_debug is, or trace out what it is used for can be helped by having a more meaningful name.
When it comes to the generic name property - that is really an overloaded term and seems easy to confuse with a system / configuration property. Having a distinct term used with FATEs could improve understanding - rather than needing to know which context a "property" is being used, at least in FATEs it would be clearer - assuming that we can find a suitable substitute that does not add more confusion than it helps.
--
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 closed issue #2841: Change ZooStore get/set property methods to use enum instead of String for property name
Posted by GitBox <gi...@apache.org>.
milleruntime closed issue #2841: Change ZooStore get/set property methods to use enum instead of String for property name
URL: https://github.com/apache/accumulo/issues/2841
--
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] cshannon commented on issue #2841: Change ZooStore get/set property methods to use enum instead of String for property name
Posted by GitBox <gi...@apache.org>.
cshannon commented on issue #2841:
URL: https://github.com/apache/accumulo/issues/2841#issuecomment-1203046145
Can someone assign the issue to me? I can take a look at it.
--
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] EdColeman commented on issue #2841: Change ZooStore get/set property methods to use enum instead of String for property name
Posted by GitBox <gi...@apache.org>.
EdColeman commented on issue #2841:
URL: https://github.com/apache/accumulo/issues/2841#issuecomment-1203218688
There should not be an issue with backward compatibility and previously stored data. When performing an upgrade there should not be any outstanding FATE transactions - this includes transaction that have finished (SUCCESS or FAILED) but not yet cleaned up.
This "requirement" is because FATE transaction serialization of the operations can change between versions and there is no way to guarantee compatibility between versions. FOr this reason there cannot be any FATE transactions from previous versions.
Without any in progress FATES, any writes will be done with the new name and there will be no previous data to worry about.
However, the rename could be handled separately, but it seems that if the code is changing now it is just as efficient to change it once, now, and be done with it (clearly, my opinion)
--
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] EdColeman commented on issue #2841: Change ZooStore get/set property methods to use enum instead of String for property name
Posted by GitBox <gi...@apache.org>.
EdColeman commented on issue #2841:
URL: https://github.com/apache/accumulo/issues/2841#issuecomment-1202700245
See[ #2829 ](https://github.com/apache/accumulo/pull/2829) for a discussion on changing the name of prop_debug to something that reflects what is stored, vs how it can be used.
--
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] cshannon commented on issue #2841: Change ZooStore get/set property methods to use enum instead of String for property name
Posted by GitBox <gi...@apache.org>.
cshannon commented on issue #2841:
URL: https://github.com/apache/accumulo/issues/2841#issuecomment-1203232550
Yeah, if the consensus is that the name "property" is a bad name about what is store and something like "node data" is better than might as well just change everything now with the API including the enum names, method names, etc. My main concern was breaking existing data but if that is not an issue as you pointed out then I don't see why not to rename it all now which I can do.
--
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