Skip to content

Commit 00949f0

Browse files
komamitsukomamitsu
authored
Refactoring CoordinatorGroupCommitKeyManipulator by moving the implementation to DefaultGroupCommitKeyManipulator to improve the reusability (#2389)
1 parent ea156cc commit 00949f0

File tree

16 files changed

+137
-121
lines changed

16 files changed

+137
-121
lines changed

‎core/src/main/java/com/scalar/db/transaction/consensuscommit/Coordinator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
import com.scalar.db.io.Key;
2121
import com.scalar.db.io.Value;
2222
import com.scalar.db.transaction.consensuscommit.CoordinatorGroupCommitter.CoordinatorGroupCommitKeyManipulator;
23-
import com.scalar.db.util.groupcommit.KeyManipulator.Keys;
23+
import com.scalar.db.util.groupcommit.GroupCommitKeyManipulator.Keys;
2424
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
2525
import java.util.ArrayList;
2626
import java.util.Collections;

‎core/src/main/java/com/scalar/db/transaction/consensuscommit/CoordinatorGroupCommitter.java‎

Lines changed: 3 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
package com.scalar.db.transaction.consensuscommit;
22

3+
import com.scalar.db.util.groupcommit.DefaultGroupCommitKeyManipulator;
34
import com.scalar.db.util.groupcommit.GroupCommitConfig;
45
import com.scalar.db.util.groupcommit.GroupCommitter;
5-
import com.scalar.db.util.groupcommit.KeyManipulator;
66
import java.util.Optional;
7-
import java.util.concurrent.ThreadLocalRandom;
87

98
public class CoordinatorGroupCommitter
109
extends GroupCommitter<String, String, String, String, String, Snapshot> {
@@ -35,93 +34,7 @@ public static boolean isEnabled(ConsensusCommitConfig config) {
3534
return config.isCoordinatorGroupCommitEnabled();
3635
}
3736

37+
// The behavior of this class is completely the same as the parent class for now.
3838
public static class CoordinatorGroupCommitKeyManipulator
39-
implements KeyManipulator<String, String, String, String, String> {
40-
private static final int PRIMARY_KEY_SIZE = 24;
41-
private static final char DELIMITER = '$';
42-
private static final int MAX_FULL_KEY_SIZE = 64;
43-
private static final int MAX_CHILD_KEY_SIZE =
44-
MAX_FULL_KEY_SIZE - PRIMARY_KEY_SIZE - 1 /* delimiter */;
45-
private static final char[] CHARS_FOR_PRIMARY_KEY;
46-
private static final int CHARS_FOR_PRIMARY_KEY_SIZE;
47-
48-
static {
49-
int digitsLen = '9' - '0' + 1;
50-
int upperCasesLen = 'Z' - 'A' + 1;
51-
int lowerCasesLen = 'z' - 'a' + 1;
52-
CHARS_FOR_PRIMARY_KEY = new char[digitsLen + upperCasesLen + lowerCasesLen];
53-
54-
int index = 0;
55-
for (char c = '0'; c <= '9'; c++) {
56-
CHARS_FOR_PRIMARY_KEY[index++] = c;
57-
}
58-
for (char c = 'A'; c <= 'Z'; c++) {
59-
CHARS_FOR_PRIMARY_KEY[index++] = c;
60-
}
61-
for (char c = 'a'; c <= 'z'; c++) {
62-
CHARS_FOR_PRIMARY_KEY[index++] = c;
63-
}
64-
65-
CHARS_FOR_PRIMARY_KEY_SIZE = CHARS_FOR_PRIMARY_KEY.length;
66-
}
67-
68-
@Override
69-
public String generateParentKey() {
70-
char[] chars = new char[PRIMARY_KEY_SIZE];
71-
for (int i = 0; i < PRIMARY_KEY_SIZE; i++) {
72-
chars[i] =
73-
CHARS_FOR_PRIMARY_KEY[ThreadLocalRandom.current().nextInt(CHARS_FOR_PRIMARY_KEY_SIZE)];
74-
}
75-
return new String(chars);
76-
}
77-
78-
@Override
79-
public String fullKey(String parentKey, String childKey) {
80-
if (parentKey.length() != PRIMARY_KEY_SIZE) {
81-
throw new IllegalArgumentException(
82-
String.format(
83-
"The length of parent key must be %d. ParentKey: %s", PRIMARY_KEY_SIZE, childKey));
84-
}
85-
if (childKey.length() > MAX_CHILD_KEY_SIZE) {
86-
throw new IllegalArgumentException(
87-
String.format(
88-
"The length of child key must not exceed %d. ChildKey: %s",
89-
MAX_CHILD_KEY_SIZE, childKey));
90-
}
91-
return parentKey + DELIMITER + childKey;
92-
}
93-
94-
@Override
95-
public boolean isFullKey(Object obj) {
96-
if (!(obj instanceof String)) {
97-
return false;
98-
}
99-
String key = (String) obj;
100-
return key.length() > PRIMARY_KEY_SIZE && key.charAt(PRIMARY_KEY_SIZE) == DELIMITER;
101-
}
102-
103-
@Override
104-
public Keys<String, String, String> keysFromFullKey(String fullKey) {
105-
if (!isFullKey(fullKey)) {
106-
throw new IllegalArgumentException("Invalid full key. Key:" + fullKey);
107-
}
108-
109-
return new Keys<>(
110-
fullKey.substring(0, PRIMARY_KEY_SIZE),
111-
fullKey.substring(PRIMARY_KEY_SIZE + 1 /* delimiter */),
112-
fullKey);
113-
}
114-
115-
@Override
116-
public String emitFullKeyFromFullKey(String fullKey) {
117-
// Return the string as is since the value is already String.
118-
return fullKey;
119-
}
120-
121-
@Override
122-
public String emitParentKeyFromParentKey(String parentKey) {
123-
// Return the string as is since the value is already String.
124-
return parentKey;
125-
}
126-
}
39+
extends DefaultGroupCommitKeyManipulator {}
12740
}

‎core/src/main/java/com/scalar/db/util/groupcommit/DefaultGroupCommitKeyManipulator.java‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
package com.scalar.db.util.groupcommit;
2+
3+
import java.util.concurrent.ThreadLocalRandom;
4+
5+
public class DefaultGroupCommitKeyManipulator
6+
implements GroupCommitKeyManipulator<String, String, String, String, String> {
7+
private static final int PRIMARY_KEY_SIZE = 24;
8+
private static final char DELIMITER = '$';
9+
private static final int MAX_FULL_KEY_SIZE = 64;
10+
private static final int MAX_CHILD_KEY_SIZE =
11+
MAX_FULL_KEY_SIZE - PRIMARY_KEY_SIZE - 1 /* delimiter */;
12+
private static final char[] CHARS_FOR_PRIMARY_KEY;
13+
private static final int CHARS_FOR_PRIMARY_KEY_SIZE;
14+
15+
static {
16+
int digitsLen = '9' - '0' + 1;
17+
int upperCasesLen = 'Z' - 'A' + 1;
18+
int lowerCasesLen = 'z' - 'a' + 1;
19+
CHARS_FOR_PRIMARY_KEY = new char[digitsLen + upperCasesLen + lowerCasesLen];
20+
21+
int index = 0;
22+
for (char c = '0'; c <= '9'; c++) {
23+
CHARS_FOR_PRIMARY_KEY[index++] = c;
24+
}
25+
for (char c = 'A'; c <= 'Z'; c++) {
26+
CHARS_FOR_PRIMARY_KEY[index++] = c;
27+
}
28+
for (char c = 'a'; c <= 'z'; c++) {
29+
CHARS_FOR_PRIMARY_KEY[index++] = c;
30+
}
31+
32+
CHARS_FOR_PRIMARY_KEY_SIZE = CHARS_FOR_PRIMARY_KEY.length;
33+
}
34+
35+
@Override
36+
public String generateParentKey() {
37+
char[] chars = new char[PRIMARY_KEY_SIZE];
38+
for (int i = 0; i < PRIMARY_KEY_SIZE; i++) {
39+
chars[i] =
40+
CHARS_FOR_PRIMARY_KEY[ThreadLocalRandom.current().nextInt(CHARS_FOR_PRIMARY_KEY_SIZE)];
41+
}
42+
return new String(chars);
43+
}
44+
45+
@Override
46+
public String fullKey(String parentKey, String childKey) {
47+
if (parentKey.length() != PRIMARY_KEY_SIZE) {
48+
throw new IllegalArgumentException(
49+
String.format(
50+
"The length of parent key must be %d. ParentKey: %s", PRIMARY_KEY_SIZE, childKey));
51+
}
52+
if (childKey.length() > MAX_CHILD_KEY_SIZE) {
53+
throw new IllegalArgumentException(
54+
String.format(
55+
"The length of child key must not exceed %d. ChildKey: %s",
56+
MAX_CHILD_KEY_SIZE, childKey));
57+
}
58+
return parentKey + DELIMITER + childKey;
59+
}
60+
61+
@Override
62+
public boolean isFullKey(Object obj) {
63+
if (!(obj instanceof String)) {
64+
return false;
65+
}
66+
String key = (String) obj;
67+
return key.length() > PRIMARY_KEY_SIZE && key.charAt(PRIMARY_KEY_SIZE) == DELIMITER;
68+
}
69+
70+
@Override
71+
public Keys<String, String, String> keysFromFullKey(String fullKey) {
72+
if (!isFullKey(fullKey)) {
73+
throw new IllegalArgumentException("Invalid full key. Key:" + fullKey);
74+
}
75+
76+
return new Keys<>(
77+
fullKey.substring(0, PRIMARY_KEY_SIZE),
78+
fullKey.substring(PRIMARY_KEY_SIZE + 1 /* delimiter */),
79+
fullKey);
80+
}
81+
82+
@Override
83+
public String emitFullKeyFromFullKey(String fullKey) {
84+
// Return the string as is since the value is already String.
85+
return fullKey;
86+
}
87+
88+
@Override
89+
public String emitParentKeyFromParentKey(String parentKey) {
90+
// Return the string as is since the value is already String.
91+
return parentKey;
92+
}
93+
}

‎core/src/main/java/com/scalar/db/util/groupcommit/DelayedGroup.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ class DelayedGroup<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_K
1515
GroupCommitConfig config,
1616
FULL_KEY fullKey,
1717
Emittable<EMIT_PARENT_KEY, EMIT_FULL_KEY, V> emitter,
18-
KeyManipulator<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY>
18+
GroupCommitKeyManipulator<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY>
1919
keyManipulator) {
2020
super(emitter, keyManipulator, 1, config.oldGroupAbortTimeoutMillis());
2121
this.fullKey = fullKey;

‎core/src/main/java/com/scalar/db/util/groupcommit/Group.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ abstract class Group<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL
1414
private static final Logger logger = LoggerFactory.getLogger(Group.class);
1515

1616
protected final Emittable<EMIT_PARENT_KEY, EMIT_FULL_KEY, V> emitter;
17-
protected final KeyManipulator<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY>
17+
protected final GroupCommitKeyManipulator<
18+
PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY>
1819
keyManipulator;
1920
private final int capacity;
2021
private final AtomicReference<Integer> size = new AtomicReference<>();
@@ -61,7 +62,7 @@ enum Status {
6162

6263
Group(
6364
Emittable<EMIT_PARENT_KEY, EMIT_FULL_KEY, V> emitter,
64-
KeyManipulator<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY>
65+
GroupCommitKeyManipulator<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY>
6566
keyManipulator,
6667
int capacity,
6768
long oldGroupAbortTimeoutMillis) {

core/src/main/java/com/scalar/db/util/groupcommit/KeyManipulator.java renamed to core/src/main/java/com/scalar/db/util/groupcommit/GroupCommitKeyManipulator.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@
1515
* @param <EMIT_FULL_KEY> A full-key type that Emitter can interpret.
1616
*/
1717
@Immutable
18-
public interface KeyManipulator<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY> {
18+
public interface GroupCommitKeyManipulator<
19+
PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY> {
1920
class Keys<PARENT_KEY, CHILD_KEY, FULL_KEY> {
2021
public final PARENT_KEY parentKey;
2122
public final CHILD_KEY childKey;

‎core/src/main/java/com/scalar/db/util/groupcommit/GroupCommitter.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import com.google.common.annotations.VisibleForTesting;
44
import com.google.common.util.concurrent.Uninterruptibles;
5-
import com.scalar.db.util.groupcommit.KeyManipulator.Keys;
5+
import com.scalar.db.util.groupcommit.GroupCommitKeyManipulator.Keys;
66
import java.io.Closeable;
77
import java.time.Instant;
88
import java.util.concurrent.TimeUnit;
@@ -46,7 +46,8 @@ public class GroupCommitter<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EM
4646
@Nullable private final GroupCommitMonitor groupCommitMonitor;
4747

4848
// This contains logics of how to treat keys.
49-
private final KeyManipulator<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY>
49+
private final GroupCommitKeyManipulator<
50+
PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY>
5051
keyManipulator;
5152

5253
private final GroupManager<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY, V>
@@ -62,7 +63,7 @@ public class GroupCommitter<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EM
6263
public GroupCommitter(
6364
String label,
6465
GroupCommitConfig config,
65-
KeyManipulator<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY>
66+
GroupCommitKeyManipulator<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY>
6667
keyManipulator) {
6768
logger.info("Starting GroupCommitter. Label: {}, Config: {}", label, config);
6869
this.keyManipulator = keyManipulator;
@@ -209,7 +210,7 @@ public void close() {
209210
GroupManager<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY, V>
210211
createGroupManager(
211212
GroupCommitConfig config,
212-
KeyManipulator<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY>
213+
GroupCommitKeyManipulator<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY>
213214
keyManipulator) {
214215
return new GroupManager<>(config, keyManipulator);
215216
}

‎core/src/main/java/com/scalar/db/util/groupcommit/GroupManager.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import com.google.common.annotations.VisibleForTesting;
66
import com.google.errorprone.annotations.ThreadSafe;
77
import com.google.errorprone.annotations.concurrent.LazyInit;
8-
import com.scalar.db.util.groupcommit.KeyManipulator.Keys;
8+
import com.scalar.db.util.groupcommit.GroupCommitKeyManipulator.Keys;
99
import java.util.HashMap;
1010
import java.util.List;
1111
import java.util.Map;
@@ -49,15 +49,16 @@ class GroupManager<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_K
4949
groupCleanupWorker;
5050

5151
// Custom operations injected by the client
52-
private final KeyManipulator<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY>
52+
private final GroupCommitKeyManipulator<
53+
PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY>
5354
keyManipulator;
5455
@LazyInit private Emittable<EMIT_PARENT_KEY, EMIT_FULL_KEY, V> emitter;
5556

5657
private final GroupCommitConfig config;
5758

5859
GroupManager(
5960
GroupCommitConfig config,
60-
KeyManipulator<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY>
61+
GroupCommitKeyManipulator<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY>
6162
keyManipulator) {
6263
this.keyManipulator = keyManipulator;
6364
this.config = config;

‎core/src/main/java/com/scalar/db/util/groupcommit/NormalGroup.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ class NormalGroup<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KE
2727
NormalGroup(
2828
GroupCommitConfig config,
2929
Emittable<EMIT_PARENT_KEY, EMIT_FULL_KEY, V> emitter,
30-
KeyManipulator<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY>
30+
GroupCommitKeyManipulator<PARENT_KEY, CHILD_KEY, FULL_KEY, EMIT_PARENT_KEY, EMIT_FULL_KEY>
3131
keyManipulator) {
3232
super(emitter, keyManipulator, config.slotCapacity(), config.oldGroupAbortTimeoutMillis());
3333
this.delayedSlotMoveTimeoutMillis = config.delayedSlotMoveTimeoutMillis();

‎core/src/test/java/com/scalar/db/util/groupcommit/DelayedGroupTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,13 @@
2626
@ExtendWith(MockitoExtension.class)
2727
class DelayedGroupTest {
2828
@Mock private Emittable<String, String, Integer> emitter;
29-
private TestableKeyManipulator keyManipulator;
29+
private TestableGroupCommitKeyManipulator keyManipulator;
3030

3131
@BeforeEach
3232
void setUp() {
3333
// This generates parent keys which start with "0000" and increment by one for each subsequent
3434
// key ("0001", "0002"...).
35-
keyManipulator = new TestableKeyManipulator();
35+
keyManipulator = new TestableGroupCommitKeyManipulator();
3636
}
3737

3838
@Test

0 commit comments

Comments
 (0)