Skip to content

Commit

Permalink
remove skipRecords from fetching API and protobuf
Browse files Browse the repository at this point in the history 
AFAIK skipRecords was never used. By inspection, the code that would have been exercised if it had been used has never been correct. Removing skipRecords from the code base.

Also:
- Added a number of TODOs to the code from the reviews
- Changed some variable names
  • Loading branch information
@jayjwylie
jayjwylie committed Mar 20, 2013
1 parent d486f2a commit a2d9ebb
Show file tree
Hide file tree
Showing 13 changed files with 304 additions and 338 deletions.
18 changes: 2 additions & 16 deletions src/java/voldemort/client/protocol/admin/AdminClient.java
View file
Expand Up @@ -1444,7 +1444,6 @@ private void initiateFetchRequest(DataOutputStream outputStream,
boolean fetchValues,
boolean fetchMasterEntries,
Cluster initialCluster,
long skipRecords,
long maxRecords) throws IOException {
HashMap<Integer, List<Integer>> filteredReplicaToPartitionList = Maps.newHashMap();
if(fetchMasterEntries) {
Expand All @@ -1460,7 +1459,6 @@ private void initiateFetchRequest(DataOutputStream outputStream,
.setFetchValues(fetchValues)
.addAllReplicaToPartition(ProtoUtils.encodePartitionTuple(filteredReplicaToPartitionList))
.setStore(storeName)
.setSkipRecords(skipRecords)
.setMaxRecords(maxRecords);

try {
Expand Down Expand Up @@ -1576,7 +1574,6 @@ public Pair<ByteArray, Versioned<byte[]>> computeNext() {
* @param filter Custom filter implementation to filter out entries
* which should not be fetched.
* @param fetchMasterEntries Fetch an entry only if master replica
* @param skipRecords Number of records to skip
* @return An iterator which allows entries to be streamed as they're
* being iterated over.
*/
Expand All @@ -1585,15 +1582,13 @@ public Iterator<Pair<ByteArray, Versioned<byte[]>>> fetchEntries(int nodeId,
List<Integer> partitionList,
VoldemortFilter filter,
boolean fetchMasterEntries,
long skipRecords,
long maxRecords) {
return fetchEntries(nodeId,
storeName,
helperOps.getReplicaToPartitionMap(nodeId, storeName, partitionList),
filter,
fetchMasterEntries,
null,
skipRecords,
maxRecords);
}

Expand All @@ -1616,7 +1611,7 @@ public Iterator<Pair<ByteArray, Versioned<byte[]>>> fetchEntries(int nodeId,
List<Integer> partitionList,
VoldemortFilter filter,
boolean fetchMasterEntries) {
return fetchEntries(nodeId, storeName, partitionList, filter, fetchMasterEntries, 0, 0);
return fetchEntries(nodeId, storeName, partitionList, filter, fetchMasterEntries, 0);
}

// TODO: " HashMap<Integer, List<Integer>> replicaToPartitionList," is a
Expand Down Expand Up @@ -1652,7 +1647,6 @@ public Iterator<Pair<ByteArray, Versioned<byte[]>>> fetchEntries(int nodeId,
* decision to fetch entries. This is important during
* rebalancing where-in we want to fetch keys using an older
* metadata compared to the new one.
* @param skipRecords Number of records to skip
* @return An iterator which allows entries to be streamed as they're
* being iterated over.
*/
Expand All @@ -1662,7 +1656,6 @@ public Iterator<Pair<ByteArray, Versioned<byte[]>>> fetchEntries(int nodeId,
VoldemortFilter filter,
boolean fetchMasterEntries,
Cluster initialCluster,
long skipRecords,
long maxRecords) {

Node node = AdminClient.this.getAdminClientCluster().getNodeById(nodeId);
Expand All @@ -1681,7 +1674,6 @@ public Iterator<Pair<ByteArray, Versioned<byte[]>>> fetchEntries(int nodeId,
true,
fetchMasterEntries,
initialCluster,
skipRecords,
maxRecords);
} catch(IOException e) {
helperOps.close(sands.getSocket());
Expand Down Expand Up @@ -1800,7 +1792,6 @@ public ByteArray computeNext() {
* @param filter Custom filter implementation to filter out entries
* which should not be fetched.
* @param fetchMasterEntries Fetch a key only if master replica
* @param skipRecords Number of keys to skip
* @return An iterator which allows keys to be streamed as they're being
* iterated over.
*/
Expand All @@ -1809,15 +1800,13 @@ public Iterator<ByteArray> fetchKeys(int nodeId,
List<Integer> partitionList,
VoldemortFilter filter,
boolean fetchMasterEntries,
long skipRecords,
long maxRecords) {
return fetchKeys(nodeId,
storeName,
helperOps.getReplicaToPartitionMap(nodeId, storeName, partitionList),
filter,
fetchMasterEntries,
null,
skipRecords,
maxRecords);
}

Expand All @@ -1840,7 +1829,7 @@ public Iterator<ByteArray> fetchKeys(int nodeId,
List<Integer> partitionList,
VoldemortFilter filter,
boolean fetchMasterEntries) {
return fetchKeys(nodeId, storeName, partitionList, filter, fetchMasterEntries, 0, 0);
return fetchKeys(nodeId, storeName, partitionList, filter, fetchMasterEntries, 0);
}

/**
Expand All @@ -1855,7 +1844,6 @@ public Iterator<ByteArray> fetchKeys(int nodeId,
* @param filter Custom filter
* @param initialCluster Cluster to use for selecting a key. If null,
* use the default metadata from the metadata store
* @param skipRecords Number of records to skip [ Used for sampling ]
* @return Returns an iterator of the keys
*/
public Iterator<ByteArray> fetchKeys(int nodeId,
Expand All @@ -1864,7 +1852,6 @@ public Iterator<ByteArray> fetchKeys(int nodeId,
VoldemortFilter filter,
boolean fetchMasterEntries,
Cluster initialCluster,
long skipRecords,
long maxRecords) {
Node node = AdminClient.this.getAdminClientCluster().getNodeById(nodeId);
final SocketDestination destination = new SocketDestination(node.getHost(),
Expand All @@ -1882,7 +1869,6 @@ public Iterator<ByteArray> fetchKeys(int nodeId,
false,
fetchMasterEntries,
initialCluster,
skipRecords,
maxRecords);
} catch(IOException e) {
helperOps.close(sands.getSocket());
Expand Down

0 comments on commit a2d9ebb

Please sign in to comment.