-
Notifications
You must be signed in to change notification settings - Fork 379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WT-147: Dynamic Index creation. Use bulk=unordered #2189
Conversation
the exclusive handle to be reopened by the same session multiple times; the handle is unlocked when the reference count drops to zero. Needed for WT-147.
to insert a single index entry into apply_single_idx(). WT-147.
a given year, not just the first one found.
…we allow" This reverts commit c22f09f.
…ocked, using session flags. This does not yet work (fails some tests).
btrees that have "shared locking". Only enable this feature selectively, controlled by a session flag. For now, only creating and filling an index needs it.
Dhandles are always shared when needed.
The feature allows us to stop worrying about schema lock interactions when building LSM indexes on existing tables.
The regular worker-thread switch semantic causes problems when creating an index on an existing table.
It means the index creation code can treat LSM and non-LSM tables consistently.
WT-147 Add undocumented bulk=unordered for LSM cursors.
so that LSM files are filled in the application thread to avoid locking issues. Index cursors know that bulk=unordered only applies for the index file, not for column group files in the main table.
@agorrod, here's my latest attempt. Unfortunately, there's a different problem. This hangs in test_backup02.py during the populate. Here's a reduced test case: #!/usr/bin/env python
#
import wiredtiger, wttest
from helper import compare_files,\
complex_populate, complex_populate_lsm, simple_populate
# test_backup03.py
# Utilities: wt backup
# Test cursor backup with target URIs
class test_zz(wttest.WiredTigerTestCase):
# This test is written to test LSM hot backups: we test a simple LSM object
# and a complex LSM object, but we can't test them both at the same time
# because we need to load fast enough the merge threads catch up, and so we
# test the real database, not what the database might look like after the
# merging settles down.
#
# The way it works is we create 4 objects, only one of which is large, then
# we do a hot backup of one or more of the objects and compare the original
# to the backup to confirm the backup is correct.
pfx = 'test_zz'
# Create a large cache, otherwise this test runs quite slowly.
def setUpConnectionOpen(self, dir):
wtopen_args = 'create,cache_size=1G'
conn = wiredtiger.wiredtiger_open(dir, wtopen_args)
self.pr(`conn`)
return conn
# Test backup with targets.
def test_backup_target(self):
self.big = 1
self.list = [1]
self.tty('--')
self.tty('PART 0')
simple_populate(self, 'table:test_zz.1', 'key_format=S', 1000)
self.tty('PART 1')
simple_populate(self, 'lsm:test_zz.2', 'key_format=S', 200000)
self.tty('PART 2')
complex_populate(self, 'table:test_zz.3', 'key_format=S', 1000)
self.tty('PART 3')
complex_populate_lsm(self, 'table:test_zz.4', 'key_format=S', 1000)
self.tty('DONE populate')
# Backup needs a checkpoint
self.session.checkpoint(None)
if __name__ == '__main__':
wttest.run() Here are backtraces from the relevant threads. Thread 1 is the application thread:
The application is holding the SCHEMA_LOCK (frame 11) and wants the CHECKPOINT_LOCK (in frame 3). Thread #7 is the LSM worker thread:
The LSM worker thread has obtained the CHECKPOINT_LOCK (frame 5), and needs the SCHEMA_LOCK (frame 3). I suppose we could always grab the checkpoint lock before taking the schema lock in __wt_session_create (maybe only need to do this if we are creating an index). Any other ideas? Unfortunately, building an index and doing a checkpoint, in their own worst case, may be long operations. I wonder if there's a way to build the index file first as sort of a temp file (outside of the WT schema), and when that is finished, then take the schema lock and officially insert it? We'd want to keep a lock on the main file so inserts wouldn't happen in the meantime. That's a bigger project, perhaps as a followup. |
It no longer enforces exclusive access to the chunk.
@ddanderson I wasn't able to reproduce the hang you reported. I've pushed a change that relaxes the |
@agorrod Thanks! Perhaps you didn't see the same hang because of timing of asynchronous checkpoints. I didn't see it if I changed the line: At any rate, your change fixes my hang, I'm bumping all the number of entries way up now to make sure. |
@agorrod I've done some extra testing and I'm satisfied with this. |
@@ -331,6 +331,11 @@ __wt_conn_btree_open( | |||
F_SET(btree, LF_ISSET(WT_BTREE_SPECIAL_FLAGS)); | |||
|
|||
WT_ERR(__wt_btree_open(session, cfg)); | |||
if (F_ISSET(dhandle, WT_DHANDLE_EXCLUSIVE) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ddanderson I don't really understand the condition here. Could you add a comment to explain why it's necessary?
@ddanderson I had a couple of minor questions, but this is generally looking good to me now. Well done! |
@agorrod, I tightened up the conditional and commented the best I could. If a dhandle used by a bulk cursor is allowed to be shared, then
I haven't gotten very far at looking at this, and I don't have any deeper understanding of why a bulk dhandle cannot use the sharing code. If you do, maybe we can make the comment more insightful. |
@ddanderson Sorry I didn't get back to this today. It's top of my list next week. |
…nglements with the WT_CURSOR_INDEX implementation, which is concerned with reading from indexes.
@ddanderson, after reviewing where this branch ended up, I pushed cfd31ce in an attempt to simplify things. This populates the index source directly (usually "file:foo" or "lsm:foo" rather than "index:foo"). I think this means we don't need to filter out flags because we avoid the issue of an index cursor opening the column groups. Can you please review? @agorrod is otherwise happy, I'll do a final review and merge if you agree with this approach. |
@michaelcahill LGTM, that's a very nice simplification. |
@ddanderson I'm happy with this change now. @michaelcahill Would you like to do a final review before merging this change? |
Thanks @ddanderson -- congratulations on seeing this through! I'll merge now... |
WT-147: Dynamic Index creation. Use bulk=unordered
Woo! Thanks for your patience with my review turnarounds @ddanderson |
@agorrod and @michaelcahill, thanks guys! Feels good to get it done. |
No description provided.