You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by hnfgns <gi...@git.apache.org> on 2016/01/15 22:45:09 UTC
[GitHub] drill pull request: DRILL-4275: Refactor e/pstore interfaces and t...
GitHub user hnfgns opened a pull request:
https://github.com/apache/drill/pull/325
DRILL-4275: Refactor e/pstore interfaces and their factories to provide a unified mechanism to access stores
The main changes are the following:
Remove EStore and EStoreProvider interfaces.
Rename PStore to Store, PStoreProvider to StoreProvider
Extend Store with getMode():StoreMode that returns store persistence level
Ensure StoreProvider.getStore(mode) acts like a factory.
Ensure Store implementations throw StoreException in case of failures and handle StoreExceptions
All other files contain very tiny, local, satellite changes like variable name updates for name consistency, exception handling et cedera.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hnfgns/incubator-drill DRILL-4275
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/325.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #325
----
commit 5b5dcd52242261050cfac2dfd2bbfc64e103cf12
Author: Hanifi Gunes <ha...@gmail.com>
Date: 2016-01-15T01:06:21Z
DRILL-4275: Refactor e/pstore interfaces and their factories to provide a unified mechanism to access stores
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: Refactor e/pstore interfaces and t...
Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on the pull request:
https://github.com/apache/drill/pull/325#issuecomment-172369155
You want to do a hangout to discuss the concept of EStores and watches and the relationship with the ClusterCoordinator interface. We're currently actively in the process of implementing a new EStore/PStore pair and I'd like to figure out the best way to improve the interface without major disruption to the work we're doing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: Refactor e/pstore interfaces and t...
Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on the pull request:
https://github.com/apache/drill/pull/325#issuecomment-172747355
Let's do a separate hangout. This seems to deep/specific to cover in a
large group.
On Jan 18, 2016 9:51 AM, "Hanifi Gunes" <no...@github.com> wrote:
> Cool. Let's sync up on tomorrow's weekly hangout. Let me know if that does
> not work for you.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/drill/pull/325#issuecomment-172604790>.
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: Refactor e/pstore interfaces and t...
Posted by hnfgns <gi...@git.apache.org>.
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/325#discussion_r49916936
--- Diff: contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoGroupScan.java ---
@@ -180,39 +179,39 @@ private void init() throws IOException {
chunksMapping = Maps.newHashMap();
chunksInverseMapping = Maps.newLinkedHashMap();
if (isShardedCluster(client)) {
- MongoDatabase db = client.getDatabase(CONFIG);
- MongoCollection<Document> chunksCollection = db.getCollection(CHUNKS);
+ MongoDatabase db = client.getDatabase(DrillMongoConstants.CONFIG);
--- End diff --
Whole point here was to avoid using an interface for constants and to avoid doing a static/wildcard import, following [Google guidelines](
https://google.github.io/styleguide/javaguide.html#s3.3.1-wildcard-imports). However, if this is taken as a show stopper, I can switch to static imports, disliking myself later ;)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: Refactor e/pstore interfaces and t...
Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on the pull request:
https://github.com/apache/drill/pull/325#issuecomment-172106187
I don't think we should change the PStore API without some level of discussion since it was designed as a public API. Can you share the driving requirements that pushed you to propose these changes? I also think renaming PStore to PersistentStore could cause user confusion since PersistentStore is very generic and could be mistaken for some place where people can store tables.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: Refactor e/pstore interfaces and t...
Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:
https://github.com/apache/drill/pull/325#discussion_r49909762
--- Diff: contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoGroupScan.java ---
@@ -180,39 +179,39 @@ private void init() throws IOException {
chunksMapping = Maps.newHashMap();
chunksInverseMapping = Maps.newLinkedHashMap();
if (isShardedCluster(client)) {
- MongoDatabase db = client.getDatabase(CONFIG);
- MongoCollection<Document> chunksCollection = db.getCollection(CHUNKS);
+ MongoDatabase db = client.getDatabase(DrillMongoConstants.CONFIG);
--- End diff --
Can you use a static import here so we don't have a bunch of extra noise in the patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: Refactor e/pstore interfaces and t...
Posted by hnfgns <gi...@git.apache.org>.
Github user hnfgns commented on the pull request:
https://github.com/apache/drill/pull/325#issuecomment-172126518
I took the API somewhat Drill internal but you are right about discussing this in the public list first. This patch -as the numbers above check out- is more of a clean-up than an extension. My plan was to discuss possible changes to API going forward once this clean-up patch made its way to master. Here is the rationale: I spent quite a while trying to understand the current design, event flow in the API in an attempt to find a way for subscribing to store events. Especially being notified when an ephemeral node dies just like what Zookeeper watches offer. I got confused multiple times with E/PStore and E/PStoreProvider distinction at interface level as it seems redundant. That's how the idea of this clean-up patch rolled out.
As for naming, I am open for non-abbreviated alternates. I would invite developers relying on this interface to read the documentation first. To this end, I will go ahead and extend documentation with possible warning areas.
Please let me know if you want me to address specific issues with this patch.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: Refactor e/pstore interfaces and t...
Posted by hnfgns <gi...@git.apache.org>.
Github user hnfgns closed the pull request at:
https://github.com/apache/drill/pull/325
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: Refactor e/pstore interfaces and t...
Posted by hnfgns <gi...@git.apache.org>.
Github user hnfgns commented on the pull request:
https://github.com/apache/drill/pull/325#issuecomment-172604790
Cool. Let's sync up on tomorrow's weekly hangout. Let me know if that does not work for you.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: Refactor e/pstore interfaces and t...
Posted by hnfgns <gi...@git.apache.org>.
Github user hnfgns commented on the pull request:
https://github.com/apache/drill/pull/325#issuecomment-174113221
I have created the following [doodle](http://doodle.com/poll/zxkxdzfkgfuanw59) to schedule this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4275: Refactor e/pstore interfaces and t...
Posted by hnfgns <gi...@git.apache.org>.
Github user hnfgns commented on the pull request:
https://github.com/apache/drill/pull/325#issuecomment-173003718
How about anytime tomorrow from 10am till noon?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---