Skip to content

Commit 2dc553b

Browse files
spetruniaspetrunia
committed
Code cleanup: Make LockingIterator's methods accept direction as parameter
.. not as template argument.
1 parent e1aee41 commit 2dc553b

File tree

2 files changed

+182
-176
lines changed

2 files changed

+182
-176
lines changed

‎storage/rocksdb/rdb_locking_iter.cc‎

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,182 @@ void LockingIterator::Prev() {
7474
ScanBackward(rocksdb::Slice(current_key), true);
7575
}
7676

77+
/*
78+
@brief
79+
Lock range from target to end_key.
80+
81+
@detail
82+
In forward-ordered scan, target < end_key. In backward-ordered scan, it's
83+
other way around.
84+
85+
We might have already locked a subset of this range, a subrange that
86+
starts from target and extends to some point between target and end_key.
87+
*/
88+
void LockingIterator::lock_up_to(bool scan_forward,
89+
const rocksdb::Slice& target,
90+
const rocksdb::Slice& end_key) {
91+
const int inv = scan_forward ? 1 : -1;
92+
auto cmp= m_cfh->GetComparator();
93+
bool endp_arg= m_kd->m_is_reverse_cf;
94+
95+
if (m_have_locked_until &&
96+
cmp->Compare(end_key, rocksdb::Slice(m_locked_until))*inv <= 0) {
97+
// We've already locked this range. The following has happened:
98+
// - m_iter->key() returned $KEY
99+
// - other transaction(s) have inserted row $ROW before the $KEY.
100+
// - we got a range lock on [range_start, $KEY]
101+
// - we've read $ROW and returned.
102+
// Now, we're looking to lock [$ROW, $KEY] but we don't need to,
103+
// we already have a lock on this range.
104+
} else {
105+
if (scan_forward) {
106+
m_status = m_txn->GetRangeLock(m_cfh,
107+
rocksdb::Endpoint(target, endp_arg),
108+
rocksdb::Endpoint(end_key, endp_arg));
109+
} else {
110+
m_status = m_txn->GetRangeLock(m_cfh,
111+
rocksdb::Endpoint(end_key, endp_arg),
112+
rocksdb::Endpoint(target, endp_arg));
113+
}
114+
115+
if (!m_status.ok())
116+
return;
117+
118+
// Save the bound where we locked until:
119+
m_have_locked_until= true;
120+
m_locked_until.assign(end_key.data(), end_key.size());
121+
if (m_lock_count) (*m_lock_count)++;
122+
}
123+
}
124+
125+
/*
126+
Lock the range from target till the iterator end point that we are scaning
127+
towards. If there's no iterator bound, use index start (or end, depending
128+
on the scan direction)
129+
*/
130+
void LockingIterator::lock_till_iterator_end(bool scan_forward,
131+
const rocksdb::Slice& target) {
132+
rocksdb::Slice end;
133+
uchar buf[Rdb_key_def::INDEX_NUMBER_SIZE];
134+
uint size;
135+
if (scan_forward) {
136+
if (m_read_opts.iterate_upper_bound)
137+
end = *m_read_opts.iterate_upper_bound;
138+
else {
139+
if (m_kd->m_is_reverse_cf)
140+
m_kd->get_infimum_key(buf, &size);
141+
else
142+
m_kd->get_supremum_key(buf, &size);
143+
144+
DBUG_ASSERT(size == Rdb_key_def::INDEX_NUMBER_SIZE);
145+
end = rocksdb::Slice((const char*)buf, size);
146+
}
147+
} else {
148+
if (m_read_opts.iterate_lower_bound)
149+
end = *m_read_opts.iterate_lower_bound;
150+
else {
151+
if (m_kd->m_is_reverse_cf)
152+
m_kd->get_supremum_key(buf, &size);
153+
else
154+
m_kd->get_infimum_key(buf, &size);
155+
156+
DBUG_ASSERT(size == Rdb_key_def::INDEX_NUMBER_SIZE);
157+
end = rocksdb::Slice((const char*)buf, size);
158+
}
159+
}
160+
// This will set m_status accordingly
161+
lock_up_to(scan_forward, target, end);
162+
}
163+
164+
/*
165+
Lock the range between [target, (current m_iter position)] and position
166+
the iterator on the first record in it.
167+
168+
@param call_next false means current iterator position is achieved by
169+
calling Seek(target).
170+
true means one also needs to call Next()
171+
*/
172+
void LockingIterator::Scan(bool scan_forward,
173+
const rocksdb::Slice& target, bool call_next) {
174+
if (!m_iter->Valid()) {
175+
m_status = m_iter->status();
176+
m_valid = false;
177+
if (m_status.ok()) {
178+
// m_iter has reached EOF
179+
lock_till_iterator_end(scan_forward, target);
180+
}
181+
return;
182+
}
183+
184+
while (1) {
185+
186+
DEBUG_SYNC(my_core::thd_get_current_thd(), "rocksdb.locking_iter_scan");
187+
188+
if (my_core::thd_killed(current_thd)) {
189+
m_status = rocksdb::Status::Aborted();
190+
m_valid = false;
191+
return;
192+
}
193+
194+
const int inv = scan_forward ? 1 : -1;
195+
auto cmp= m_cfh->GetComparator();
196+
197+
auto end_key = m_iter->key();
198+
std::string end_key_copy= end_key.ToString();
199+
200+
lock_up_to(scan_forward, target, end_key);
201+
if (!m_status.ok()) {
202+
// Failed to get a lock (most likely lock wait timeout)
203+
m_valid = false;
204+
return;
205+
}
206+
207+
//Ok, now we have a lock which is inhibiting modifications in the range
208+
// Somebody might have done external modifications, though:
209+
// - removed the key we've found
210+
// - added a key before that key.
211+
212+
// First, refresh the iterator:
213+
delete m_iter;
214+
m_iter = m_txn->GetIterator(m_read_opts, m_cfh);
215+
216+
// Then, try seeking to the same row
217+
if (scan_forward)
218+
m_iter->Seek(target);
219+
else
220+
m_iter->SeekForPrev(target);
221+
222+
if (call_next && m_iter->Valid() &&
223+
!cmp->Compare(m_iter->key(), target)) {
224+
if (scan_forward)
225+
m_iter->Next();
226+
else
227+
m_iter->Prev();
228+
}
229+
230+
if (m_iter->Valid()) {
231+
if (cmp->Compare(m_iter->key(), rocksdb::Slice(end_key_copy))*inv <= 0) {
232+
// Ok, the found key is within the locked range.
233+
m_status = rocksdb::Status::OK();
234+
m_valid= true;
235+
break;
236+
} else {
237+
// We've got a key but it is outside the range we've locked.
238+
// Re-try the lock-and-read step.
239+
continue;
240+
}
241+
} else {
242+
// Some error
243+
m_valid = false;
244+
m_status = m_iter->status();
245+
if (m_status.ok()) {
246+
// m_iter has reached EOF
247+
lock_till_iterator_end(scan_forward, target);
248+
}
249+
break;
250+
}
251+
}
252+
}
77253

78254
/*
79255
@detail

‎storage/rocksdb/rdb_locking_iter.h‎

Lines changed: 6 additions & 176 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,6 @@
1010

1111
namespace myrocks {
1212

13-
//////////////////////////////////////////////////////////////////////////////
14-
// Locking iterator
15-
//////////////////////////////////////////////////////////////////////////////
16-
1713
//
1814
// LockingIterator is an iterator that locks the rows before returning, as well
1915
// as scanned gaps between the rows.
@@ -100,183 +96,17 @@ class LockingIterator : public rocksdb::Iterator {
10096
}
10197

10298
private:
103-
/*
104-
@brief
105-
Lock range from target to end_key.
106-
107-
@detail
108-
In forward-ordered scan, target < end_key. In backward-ordered scan, it's
109-
vice versa.
110-
111-
We might have already locked a subset of this range, a subrange that
112-
starts from target and extends to some point between target and end_key.
113-
*/
114-
template <bool forward> void lock_up_to(const rocksdb::Slice& target,
115-
const rocksdb::Slice& end_key) {
116-
const int inv = forward ? 1 : -1;
117-
auto cmp= m_cfh->GetComparator();
118-
bool endp_arg= m_kd->m_is_reverse_cf;
119-
120-
if (m_have_locked_until &&
121-
cmp->Compare(end_key, rocksdb::Slice(m_locked_until))*inv <= 0) {
122-
// We've already locked this range. The following has happened:
123-
// - m_iter->key() returned $KEY
124-
// - other transaction(s) have inserted row $ROW before the $KEY.
125-
// - we got a range lock on [range_start, $KEY]
126-
// - we've read $ROW and returned.
127-
// Now, we're looking to lock [$ROW, $KEY] but we don't need to,
128-
// we already have a lock on this range.
129-
} else {
130-
if (forward) {
131-
m_status = m_txn->GetRangeLock(m_cfh,
132-
rocksdb::Endpoint(target, endp_arg),
133-
rocksdb::Endpoint(end_key, endp_arg));
134-
} else {
135-
m_status = m_txn->GetRangeLock(m_cfh,
136-
rocksdb::Endpoint(end_key, endp_arg),
137-
rocksdb::Endpoint(target, endp_arg));
138-
}
139-
140-
if (!m_status.ok())
141-
return;
142-
143-
// Save the bound where we locked until:
144-
m_have_locked_until= true;
145-
m_locked_until.assign(end_key.data(), end_key.size());
146-
if (m_lock_count) (*m_lock_count)++;
147-
}
148-
}
149-
150-
template <bool forward>
151-
void lock_till_iterator_end(const rocksdb::Slice& target) {
152-
rocksdb::Slice end;
153-
uchar buf[Rdb_key_def::INDEX_NUMBER_SIZE];
154-
uint size;
155-
if (forward) {
156-
if (m_read_opts.iterate_upper_bound)
157-
end = *m_read_opts.iterate_upper_bound;
158-
else {
159-
if (m_kd->m_is_reverse_cf)
160-
m_kd->get_infimum_key(buf, &size);
161-
else
162-
m_kd->get_supremum_key(buf, &size);
163-
164-
DBUG_ASSERT(size == Rdb_key_def::INDEX_NUMBER_SIZE);
165-
end = rocksdb::Slice((const char*)buf, size);
166-
}
167-
} else {
168-
if (m_read_opts.iterate_lower_bound)
169-
end = *m_read_opts.iterate_lower_bound;
170-
else {
171-
if (m_kd->m_is_reverse_cf)
172-
m_kd->get_supremum_key(buf, &size);
173-
else
174-
m_kd->get_infimum_key(buf, &size);
175-
176-
DBUG_ASSERT(size == Rdb_key_def::INDEX_NUMBER_SIZE);
177-
end = rocksdb::Slice((const char*)buf, size);
178-
}
179-
}
180-
// This will set m_status accordingly
181-
lock_up_to<forward>(target, end);
182-
}
183-
184-
/*
185-
Lock the range between [target, (current m_iter position)] and position
186-
the iterator on the first record in it.
187-
188-
@param call_next false means current iterator position is achieved by
189-
calling Seek(target).
190-
true means one also needs to call Next()
191-
*/
192-
template <bool forward> void Scan(const rocksdb::Slice& target,
193-
bool call_next) {
194-
if (!m_iter->Valid()) {
195-
m_status = m_iter->status();
196-
m_valid = false;
197-
if (m_status.ok()) {
198-
// m_iter has reached EOF
199-
lock_till_iterator_end<forward>(target);
200-
}
201-
return;
202-
}
203-
204-
while (1) {
205-
206-
DEBUG_SYNC(my_core::thd_get_current_thd(), "rocksdb.locking_iter_scan");
207-
208-
if (my_core::thd_killed(current_thd)) {
209-
m_status = rocksdb::Status::Aborted();
210-
m_valid = false;
211-
return;
212-
}
213-
214-
const int inv = forward ? 1 : -1;
215-
auto cmp= m_cfh->GetComparator();
216-
217-
auto end_key = m_iter->key();
218-
std::string end_key_copy= end_key.ToString();
219-
220-
lock_up_to<forward>(target, end_key);
221-
if (!m_status.ok()) {
222-
// Failed to get a lock (most likely lock wait timeout)
223-
m_valid = false;
224-
return;
225-
}
226-
227-
//Ok, now we have a lock which is inhibiting modifications in the range
228-
// Somebody might have done external modifications, though:
229-
// - removed the key we've found
230-
// - added a key before that key.
231-
232-
// First, refresh the iterator:
233-
delete m_iter;
234-
m_iter = m_txn->GetIterator(m_read_opts, m_cfh);
235-
236-
// Then, try seeking to the same row
237-
if (forward)
238-
m_iter->Seek(target);
239-
else
240-
m_iter->SeekForPrev(target);
241-
242-
if (call_next && m_iter->Valid() &&
243-
!cmp->Compare(m_iter->key(), target)) {
244-
if (forward)
245-
m_iter->Next();
246-
else
247-
m_iter->Prev();
248-
}
249-
250-
if (m_iter->Valid()) {
251-
if (cmp->Compare(m_iter->key(), rocksdb::Slice(end_key_copy))*inv <= 0) {
252-
// Ok, the found key is within the locked range.
253-
m_status = rocksdb::Status::OK();
254-
m_valid= true;
255-
break;
256-
} else {
257-
// We've got a key but it is outside the range we've locked.
258-
// Re-try the lock-and-read step.
259-
continue;
260-
}
261-
} else {
262-
// Some error
263-
m_valid = false;
264-
m_status = m_iter->status();
265-
if (m_status.ok()) {
266-
// m_iter has reached EOF
267-
lock_till_iterator_end<forward>(target);
268-
}
269-
break;
270-
}
271-
}
272-
}
99+
void lock_up_to(bool scan_forward, const rocksdb::Slice& target,
100+
const rocksdb::Slice& end_key);
101+
void lock_till_iterator_end(bool scan_forward, const rocksdb::Slice& target);
102+
void Scan(bool scan_forward, const rocksdb::Slice& target, bool call_next);
273103

274104
inline void ScanForward(const rocksdb::Slice& target, bool call_next) {
275-
Scan<true>(target, call_next);
105+
Scan(true, target, call_next);
276106
}
277107

278108
inline void ScanBackward(const rocksdb::Slice& target, bool call_next) {
279-
Scan<false>(target, call_next);
109+
Scan(false, target, call_next);
280110
}
281111
};
282112

0 commit comments

Comments
 (0)