cmccabe commented on PR #13280:
URL: https://github.com/apache/kafka/pull/13280#issuecomment-1499641310
Thanks for working on this, @rondagostino and @showuon ! I think this will
be a really cool improvement.
A note about namespace names:
Rather than having an abbreviated namespace like `org.apache.kafka.pcoll` ,
how about `org.apache.kafka.server.persistent` ? This emphasizes that the code
is in `server-common` and is more consistent with our other namespace names,
which mostly don't use abbreviations.
I don't have any objection to wrapping pcollections so that you can swap in
a different library in the future. However, I don't like the wrappers here for
two main reasons:
1. They don't implement standard collection interfaces (java.util.Map,
java.util.Set)
2. They involve an additional "wrapper object" for every operation (every
addition, etc.)
The first issue is easy to fix. If you look at TimelineHashMap.java and
TimelineHashSet.java, you can see that I implemented the standard Map and Set
operations without too much fuss. It is a lot better than just implementing
`Iterable` and some getters or having awkward "convert this to a real java
collection" methods.
The second issue is harder to fix because of the nature of Java's type
system. However, I suspect that you might be able to do it by having something
like a `PCollectionFactory` that implemented `PersistentCollectionFactory`.
Then you could have methods like:
```
class PersistentCollectionFactory<K, V> {
Map<K, V> emptyMap<K, V>()
Map<K, V> singletonMap<K, V>(K k, V v)
Map<K, V> afterAdding<K, V>(Map<K, V> map, K k, V v)
Map<K, V> afterRemoving<K, V>(Map<K, V> map, K k, V v)
Map<K, V> afterRemoving<K, V>(Map<K, V> map, K k)
...
}
```
This would basically let you not have to cart around a wrapper object for
each new map you created. However you could avoid dealing with pcollections
types entirely in the calling code, and just refer to java.util.Map, etc. Since
the pcollections type do implement the standard java types, this would work
well here.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
