You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@giraph.apache.org by "Avery Ching (JIRA)" <ji...@apache.org> on 2012/08/08 08:02:09 UTC
[jira] [Created] (GIRAPH-291) PredicateLock should have a
constructor to take in a custom waiting time and additional testing
Avery Ching created GIRAPH-291:
----------------------------------
Summary: PredicateLock should have a constructor to take in a custom waiting time and additional testing
Key: GIRAPH-291
URL: https://issues.apache.org/jira/browse/GIRAPH-291
Project: Giraph
Issue Type: Improvement
Reporter: Avery Ching
Assignee: Avery Ching
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (GIRAPH-291) PredicateLock should have a
constructor to take in a custom waiting time and additional testing
Posted by "Avery Ching (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-291?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Avery Ching updated GIRAPH-291:
-------------------------------
Attachment: GIRAPH-291.2.patch
Added the Time, SystemTime, and FakeTime classes to help with testing. Integrated mocking for time into the single threaded tests.
> PredicateLock should have a constructor to take in a custom waiting time and additional testing
> -----------------------------------------------------------------------------------------------
>
> Key: GIRAPH-291
> URL: https://issues.apache.org/jira/browse/GIRAPH-291
> Project: Giraph
> Issue Type: Improvement
> Reporter: Avery Ching
> Assignee: Avery Ching
> Attachments: GIRAPH-291.2.patch, GIRAPH-291.patch
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-291) PredicateLock should have a
constructor to take in a custom waiting time and additional testing
Posted by "Eli Reisman (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13430912#comment-13430912 ]
Eli Reisman commented on GIRAPH-291:
------------------------------------
Changing waitForever to boolean is so it could be tested/verified that the call returns and the event goes through. This is why someone had the hack in the test of calling waitMsecs(-1) directly rather than testing waitForever() it doesn't happen anywhere else outside the tests.
> PredicateLock should have a constructor to take in a custom waiting time and additional testing
> -----------------------------------------------------------------------------------------------
>
> Key: GIRAPH-291
> URL: https://issues.apache.org/jira/browse/GIRAPH-291
> Project: Giraph
> Issue Type: Improvement
> Reporter: Avery Ching
> Assignee: Avery Ching
> Attachments: GIRAPH-291.patch
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-291) PredicateLock should have a
constructor to take in a custom waiting time and additional testing
Posted by "Avery Ching (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13431278#comment-13431278 ]
Avery Ching commented on GIRAPH-291:
------------------------------------
Any committers wanna take a very quick look? It's short I promise!
> PredicateLock should have a constructor to take in a custom waiting time and additional testing
> -----------------------------------------------------------------------------------------------
>
> Key: GIRAPH-291
> URL: https://issues.apache.org/jira/browse/GIRAPH-291
> Project: Giraph
> Issue Type: Improvement
> Reporter: Avery Ching
> Assignee: Avery Ching
> Attachments: GIRAPH-291.patch
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (GIRAPH-291) PredicateLock should have a
constructor to take in a custom waiting time and additional testing
Posted by "Avery Ching (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-291?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Avery Ching updated GIRAPH-291:
-------------------------------
Attachment: GIRAPH-291.3.patch
Thanks Jakob. Everything should be here.
> PredicateLock should have a constructor to take in a custom waiting time and additional testing
> -----------------------------------------------------------------------------------------------
>
> Key: GIRAPH-291
> URL: https://issues.apache.org/jira/browse/GIRAPH-291
> Project: Giraph
> Issue Type: Improvement
> Reporter: Avery Ching
> Assignee: Avery Ching
> Attachments: GIRAPH-291.2.patch, GIRAPH-291.3.patch, GIRAPH-291.patch
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-291) PredicateLock should have a
constructor to take in a custom waiting time and additional testing
Posted by "Jakob Homan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13442837#comment-13442837 ]
Jakob Homan commented on GIRAPH-291:
------------------------------------
Doesn't look like the new classes got picked up in the patch...
> PredicateLock should have a constructor to take in a custom waiting time and additional testing
> -----------------------------------------------------------------------------------------------
>
> Key: GIRAPH-291
> URL: https://issues.apache.org/jira/browse/GIRAPH-291
> Project: Giraph
> Issue Type: Improvement
> Reporter: Avery Ching
> Assignee: Avery Ching
> Attachments: GIRAPH-291.2.patch, GIRAPH-291.patch
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-291) PredicateLock should have a
constructor to take in a custom waiting time and additional testing
Posted by "Avery Ching (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13430914#comment-13430914 ]
Avery Ching commented on GIRAPH-291:
------------------------------------
I agree that testing is good, but waitForever() should be simple.
I did the "hack" you mentioned =), although in retrospect, I could have just done waitForever() and then followed it up with a waitMsecs(0) to see if the predicate did in fact change. That would be good enough for the test. I like getting rid of negative arguments in waitMsecs() that you did in GIRAPH-246, hence using it here as well.
Anyway, I think this patch is good to go and shouldn't cause any contention. This really only simplifies the code and adds more testing.
> PredicateLock should have a constructor to take in a custom waiting time and additional testing
> -----------------------------------------------------------------------------------------------
>
> Key: GIRAPH-291
> URL: https://issues.apache.org/jira/browse/GIRAPH-291
> Project: Giraph
> Issue Type: Improvement
> Reporter: Avery Ching
> Assignee: Avery Ching
> Attachments: GIRAPH-291.patch
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-291) PredicateLock should have a
constructor to take in a custom waiting time and additional testing
Posted by "Eli Reisman (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13431204#comment-13431204 ]
Eli Reisman commented on GIRAPH-291:
------------------------------------
Sorry I did not get that impression from the other threads, sounds great. I figured verifying the progress calls in the lock code would relate to trying to solve the timeout problem with the existing solution, and we already know it doesn't work, so I didn't really follow the purpose.
I believe the main contribution of 246 is making the timeouts go away until we know why they are happening. If we want to pursue the predicate lock solution, that sounds great but the annoyance of this problem is that you don't know if you've solved it without running a bunch of long jobs, and so guessing at this has proven time consuming. Once I hit something that worked, I was done guessing as I have bigger fish to fry to get Giraph to the scale I need and this seemed like an interfacing-with-Hadoop problem, not a Giraph problem.
As far as this patch, looks great to me.
> PredicateLock should have a constructor to take in a custom waiting time and additional testing
> -----------------------------------------------------------------------------------------------
>
> Key: GIRAPH-291
> URL: https://issues.apache.org/jira/browse/GIRAPH-291
> Project: Giraph
> Issue Type: Improvement
> Reporter: Avery Ching
> Assignee: Avery Ching
> Attachments: GIRAPH-291.patch
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-291) PredicateLock should have a
constructor to take in a custom waiting time and additional testing
Posted by "Eli Reisman (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13431503#comment-13431503 ]
Eli Reisman commented on GIRAPH-291:
------------------------------------
If you guys want something more comprehensive done to these tests, I'm happy to spend some time on it, I feel responsible for setting this course of action into motion anyway. Just drop me a line.
> PredicateLock should have a constructor to take in a custom waiting time and additional testing
> -----------------------------------------------------------------------------------------------
>
> Key: GIRAPH-291
> URL: https://issues.apache.org/jira/browse/GIRAPH-291
> Project: Giraph
> Issue Type: Improvement
> Reporter: Avery Ching
> Assignee: Avery Ching
> Attachments: GIRAPH-291.patch
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-291) PredicateLock should have a
constructor to take in a custom waiting time and additional testing
Posted by "Jakob Homan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13431307#comment-13431307 ]
Jakob Homan commented on GIRAPH-291:
------------------------------------
bq. Any committers wanna take a very quick look? It's short I promise!
<cough>mark it patch available!<cough>
> PredicateLock should have a constructor to take in a custom waiting time and additional testing
> -----------------------------------------------------------------------------------------------
>
> Key: GIRAPH-291
> URL: https://issues.apache.org/jira/browse/GIRAPH-291
> Project: Giraph
> Issue Type: Improvement
> Reporter: Avery Ching
> Assignee: Avery Ching
> Attachments: GIRAPH-291.patch
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-291) PredicateLock should have a
constructor to take in a custom waiting time and additional testing
Posted by "Avery Ching (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13430926#comment-13430926 ]
Avery Ching commented on GIRAPH-291:
------------------------------------
Eli, this patch is not intended to replace GIRAPH-246. I believe that the main contribution of GIRAPH-246 is to convert some waitForever() to waitMsecs() and this is good. We should also do that part of GIRAPH-246. This patch is only to address the issues I mentioned above.
1) Adds a test that proves that progress() is called multiple times in waitForever() and keeps it verified for future changes.
2) Removes some complexity around the case where waitMsecs() could wait forever (your work in GIRAPH-246).
> PredicateLock should have a constructor to take in a custom waiting time and additional testing
> -----------------------------------------------------------------------------------------------
>
> Key: GIRAPH-291
> URL: https://issues.apache.org/jira/browse/GIRAPH-291
> Project: Giraph
> Issue Type: Improvement
> Reporter: Avery Ching
> Assignee: Avery Ching
> Attachments: GIRAPH-291.patch
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-291) PredicateLock should have a
constructor to take in a custom waiting time and additional testing
Posted by "Jakob Homan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13431306#comment-13431306 ]
Jakob Homan commented on GIRAPH-291:
------------------------------------
Sleeps in tests are bad. Giraph has lots of time-based synchronization that should be tested so we should invest now in making that easy. I would recommend we -rip-off- pay homage to Voldemort's use of a [Time interface|https://github.com/jghoman/voldemort/blob/83e2957a5d5ac9c924285819f5b099a86e0f1f4a/src/java/voldemort/utils/Time.java] (with an example test [here|https://github.com/jghoman/voldemort/blob/83e2957a5d5ac9c924285819f5b099a86e0f1f4a/test/unit/voldemort/store/stats/StatsTest.java]).
> PredicateLock should have a constructor to take in a custom waiting time and additional testing
> -----------------------------------------------------------------------------------------------
>
> Key: GIRAPH-291
> URL: https://issues.apache.org/jira/browse/GIRAPH-291
> Project: Giraph
> Issue Type: Improvement
> Reporter: Avery Ching
> Assignee: Avery Ching
> Attachments: GIRAPH-291.patch
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (GIRAPH-291) PredicateLock should have a
constructor to take in a custom waiting time and additional testing
Posted by "Avery Ching (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-291?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Avery Ching updated GIRAPH-291:
-------------------------------
Attachment: GIRAPH-291.patch
This patch
1) Adds a test that proves that progress() is called multiple times in waitForever().
2) Removes some complexity around the case where waitMsecs() could wait forever (from Eli's work in GIRAPH-246).
> PredicateLock should have a constructor to take in a custom waiting time and additional testing
> -----------------------------------------------------------------------------------------------
>
> Key: GIRAPH-291
> URL: https://issues.apache.org/jira/browse/GIRAPH-291
> Project: Giraph
> Issue Type: Improvement
> Reporter: Avery Ching
> Assignee: Avery Ching
> Attachments: GIRAPH-291.patch
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-291) PredicateLock should have a
constructor to take in a custom waiting time and additional testing
Posted by "Eli Reisman (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13445312#comment-13445312 ]
Eli Reisman commented on GIRAPH-291:
------------------------------------
+1, passes 'mvn verify' etc. and committed. Cool tests! Will get 274 resolved ASAP.
> PredicateLock should have a constructor to take in a custom waiting time and additional testing
> -----------------------------------------------------------------------------------------------
>
> Key: GIRAPH-291
> URL: https://issues.apache.org/jira/browse/GIRAPH-291
> Project: Giraph
> Issue Type: Improvement
> Reporter: Avery Ching
> Assignee: Avery Ching
> Attachments: GIRAPH-291.2.patch, GIRAPH-291.3.patch, GIRAPH-291.patch
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-291) PredicateLock should have a
constructor to take in a custom waiting time and additional testing
Posted by "Eli Reisman (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13430916#comment-13430916 ]
Eli Reisman commented on GIRAPH-291:
------------------------------------
I realize we are grasping at straws here, but this approach is starting to look just like the 267 one and that one does not work. One thing I am toying with in the 246-9 version is including another call to progress in waitMsecs before the lock is locked. This was its called in the barrier waits even when they are not waitForever, at least once. It does not seem likely to make a difference, but again none of this makes sense to me as all of these solutions are very similar. I will believe and cherish any and all that will complete a long cluster job at this point.
I will attempt to run jobs (tomorrow AM probably at this point, the heavy cron job loads have begun :( for the night) and if I get any definitive answers, I will post them immediately.
Thank you for being so proactive about this everyone, it was a frustrating step backward to have to tangle with this problem again. I'm sure with you guys on the case a great solution is around the corner.
> PredicateLock should have a constructor to take in a custom waiting time and additional testing
> -----------------------------------------------------------------------------------------------
>
> Key: GIRAPH-291
> URL: https://issues.apache.org/jira/browse/GIRAPH-291
> Project: Giraph
> Issue Type: Improvement
> Reporter: Avery Ching
> Assignee: Avery Ching
> Attachments: GIRAPH-291.patch
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (GIRAPH-291) PredicateLock should have a
constructor to take in a custom waiting time and additional testing
Posted by "Eli Reisman (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/GIRAPH-291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13445037#comment-13445037 ]
Eli Reisman commented on GIRAPH-291:
------------------------------------
I will review this today and get it in, awesome. The one Eugene was referring to that is of interest now is GIRAPH-274 which handles any progress issues regarding cleanup and a few other spots we did not find problems with and did not fix here in GIRAPH-246. I am going to look into getting that one in soon (maybe today?) too for Jaeho.
> PredicateLock should have a constructor to take in a custom waiting time and additional testing
> -----------------------------------------------------------------------------------------------
>
> Key: GIRAPH-291
> URL: https://issues.apache.org/jira/browse/GIRAPH-291
> Project: Giraph
> Issue Type: Improvement
> Reporter: Avery Ching
> Assignee: Avery Ching
> Attachments: GIRAPH-291.2.patch, GIRAPH-291.3.patch, GIRAPH-291.patch
>
>
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira