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/07/26 14:51:04 UTC

[GitHub] [accumulo] milleruntime opened a new pull request, #2829: Fix name of properties in Fate

milleruntime opened a new pull request, #2829:
URL: https://github.com/apache/accumulo/pull/2829

   * Make the variables in Fate.java the full name of their ZK properties
   * Closes #2469


-- 
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 pull request #2829: Fix name of properties in Fate

Posted by GitBox <gi...@apache.org>.
milleruntime commented on PR #2829:
URL: https://github.com/apache/accumulo/pull/2829#issuecomment-1195697038

   > Is there some confusion between properties and node names, node data?
   
   The term `property` in within Fate is confusing. Yes, technically we are setting data to a ZK node. The `property` here is not the same as the Accumulo properties. Here it is an internal FATE property that is stored in ZK.


-- 
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 pull request #2829: Fix name of properties in Fate

Posted by GitBox <gi...@apache.org>.
milleruntime commented on PR #2829:
URL: https://github.com/apache/accumulo/pull/2829#issuecomment-1195730122

   > Can we rename it from property to something else then then? data, txData, even txProp could help reduce confusion, but I'd lean to something without prop in it if it reads okay in the rest of the context.
   
   Rename what? The method on the TStore interface is called `setProperty`, along with the implementing classes. I could make the logger say something like:
   <pre>
   2022-07-26T11:21:50,432 [fate.store] TRACE: [FATE[5bf40f1bd45a2bec]] setting prop_debug to CreateTable
   </pre>
   I also think we can remove the "[ ]" around the fate Tx ID now that it has a prefix.


-- 
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 pull request #2829: Fix name of properties in Fate

Posted by GitBox <gi...@apache.org>.
EdColeman commented on PR #2829:
URL: https://github.com/apache/accumulo/pull/2829#issuecomment-1195683442

   Is there some confusion between properties and node names, node data?
   
   `setProperty prop_debug to CreateTable`
   
   This is either creating the FATE prop_debug node with the data 'CreateTable` or modifying the data of an existing node.  For me, property has a stronger tie to configuration properties rather than to node names for FATES (or the transaction data stored on those nodes)  With the log statement reading `setProperty` I'd be inclined to think I could see / modify that with shell `config` command.
   


-- 
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 #2829: Fix name of properties in FateLogger

Posted by GitBox <gi...@apache.org>.
milleruntime merged PR #2829:
URL: https://github.com/apache/accumulo/pull/2829


-- 
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 pull request #2829: Fix name of properties in Fate

Posted by GitBox <gi...@apache.org>.
milleruntime commented on PR #2829:
URL: https://github.com/apache/accumulo/pull/2829#issuecomment-1195665751

   This fixes the name of the properties in ZK:
   <pre>
   [zk: localhost:2181(CONNECTED) 2] ls /accumulo/834e3388-d546-4712-b956-b0fbd6c8eba4/fate/tx_6e3fcce088dd7fe8 
   [prop_autoClean, prop_debug, repo_0000000000, repo_0000000003]
   [zk: localhost:2181(CONNECTED) 4] get /accumulo/834e3388-d546-4712-b956-b0fbd6c8eba4/fate/tx_6e3fcce088dd7fe8/prop_debug
   S CompactRange
   </pre>
   
   The property names were being printed in the log without the full name:
   <pre>
   2022-07-26T11:21:50,432 [fate.store] TRACE: [FATE[5bf40f1bd45a2bec]] setProperty debug to CreateTable
   </pre>
   
   But with this fix, the full name of the property will now be printed in the logger:
   <pre>
   2022-07-26T11:50:13,661 [fate.store] TRACE: [FATE[37fb4ecc9b07e375]] setProperty prop_debug to CreateTable
   </pre>


-- 
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 pull request #2829: Fix name of properties in Fate

Posted by GitBox <gi...@apache.org>.
EdColeman commented on PR #2829:
URL: https://github.com/apache/accumulo/pull/2829#issuecomment-1195702138

   Can we rename it from property to something else then then?  data, txData, even txProp could help reduce confusion, but I'd lean to something without prop in it if it reads okay in the rest of the context.


-- 
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 pull request #2829: Fix name of properties in Fate

Posted by GitBox <gi...@apache.org>.
EdColeman commented on PR #2829:
URL: https://github.com/apache/accumulo/pull/2829#issuecomment-1195751771

   TStore is internal API, right?  Maybe a full rename waits for the FATE API redesign, but this *could* be an incremental step towards clarifying things that help inform the future API changes. 
   
   But for just the log statement:
   
   `2022-07-26T11:21:50,432 [fate.store] TRACE: [FATE[5bf40f1bd45a2bec]] setting prop_debug to CreateTable`
   could be:
   `2022-07-26T11:21:50,432 [fate.store] TRACE: [FATE[5bf40f1bd45a2bec]] setting prop_debug node data to CreateTable`


-- 
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