You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "ctubbsii (via GitHub)" <gi...@apache.org> on 2023/12/21 21:44:47 UTC
[I] TabletManagementParameters uses convoluted serialization [accumulo]
ctubbsii opened a new issue, #4107:
URL: https://github.com/apache/accumulo/issues/4107
For #4103 , I replaced `List<Pair<Path,Path>>` with the much friendlier and more intuitive `Map<Path,Path>`.
However, when merging this into the elasticity branch, the TabletManagementIteratorIT failed. Looking at the server logs, I saw that it was failing on Gson serialization of TabletManagementParameters. In order to get it to pass, I had to preserve the `List<Pair<Path,Path>>` in the JsonData inner class, and convert it back and forth from the `Map<Path,Path>`. This is not ideal.
The TabletManagementParameters serialization should be fixed so it can use the simpler, `Map<Path,Path>` directly. I'm not sure what this involves yet, because I'm not sure why it's working with the list of pairs and not working with a map. Other maps, even of user classes, seem to deserialize just fine. And Path itself serializes fine either way. If anything, there should be fewer classes to serialize, because the removal of the Pair, so I don't understand it just yet.
See:
https://github.com/apache/accumulo/blob/48d39719148454f002ab346610ad39b911662acc/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementParameters.java#L128-L129
and
https://github.com/apache/accumulo/blob/48d39719148454f002ab346610ad39b911662acc/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementParameters.java#L248-L249
--
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
Re: [I] TabletManagementParameters uses convoluted serialization [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on issue #4107:
URL: https://github.com/apache/accumulo/issues/4107#issuecomment-1867997495
Using `List<Pair<>>` allows duplicate keys. However looking at the code its given a Map, so there would not be duplicate keys in the source data that is used to create the ` List<Pair<>>`. Seems like converting it to a map would be fine.
--
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
Re: [I] TabletManagementParameters uses convoluted serialization [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on issue #4107:
URL: https://github.com/apache/accumulo/issues/4107#issuecomment-1908981477
Closed by #4184
--
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
Re: [I] TabletManagementParameters uses convoluted serialization [accumulo]
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on issue #4107:
URL: https://github.com/apache/accumulo/issues/4107#issuecomment-1868539583
@keith-turner All of the replacements code used to be a List of pairs, which did allow duplicates, but duplicates don't make sense here. You can't have a volume replaced by two separate volumes. That would be chaos. So, I changed it all to a map in #4103 , but I had to keep this a list of pairs, because GSon threw an error in the test when deserializing the map. This code doesn't exist in the main branch, so it's new in elasticity. I'm not sure what is causing the GSon error during deserialization, and I'm not sure why serialization is needed in the elasticity branch when it's not needed in the main branch. We can't make this less convoluted until the deserialization error is fixed.
--
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
Re: [I] TabletManagementParameters uses convoluted serialization [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion closed issue #4107: TabletManagementParameters uses convoluted serialization
URL: https://github.com/apache/accumulo/issues/4107
--
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