From 4da4a5d0c8f9b0179636257e19c99a55a9ece353 Mon Sep 17 00:00:00 2001 From: Michal Karpinski Date: Thu, 12 Jan 2017 14:45:01 +0000 Subject: [PATCH] [DPM] Improvements to the network logs batch finalization mechanism The full batch will still be available to DPC if there were no network logs pending. Added some more debug logging to better investigate the issues. Test: manual for both cases - pending batch was empty and non-empty, with locally decreased timeout Test: cts-tradefed run cts --module DevicePolicyManager --test com.android.cts.devicepolicy.DeviceOwnerTest#testNetworkLoggingWithSingleUser Bug: 34245471 Bug: 29748723 Change-Id: Iee229d74d4b0a06025b305a15687f336a0aa337e --- .../android/server/devicepolicy/NetworkLogger.java | 4 +-- .../server/devicepolicy/NetworkLoggingHandler.java | 39 +++++++++++----------- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/NetworkLogger.java b/services/devicepolicy/java/com/android/server/devicepolicy/NetworkLogger.java index 49ae2bc3523a..b82cb3cfbeaf 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/NetworkLogger.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/NetworkLogger.java @@ -23,7 +23,6 @@ import android.content.pm.PackageManagerInternal; import android.net.IIpConnectivityMetrics; import android.net.INetdEventCallback; import android.os.Bundle; -import android.os.Handler; import android.os.Message; import android.os.Process; import android.os.RemoteException; @@ -115,8 +114,7 @@ final class NetworkLogger { mHandlerThread.start(); mNetworkLoggingHandler = new NetworkLoggingHandler(mHandlerThread.getLooper(), mDpm); - mNetworkLoggingHandler.scheduleBatchFinalization( - NetworkLoggingHandler.BATCH_FINALIZATION_TIMEOUT_MS); + mNetworkLoggingHandler.scheduleBatchFinalization(); mIsLoggingEnabled.set(true); return true; } else { diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/NetworkLoggingHandler.java b/services/devicepolicy/java/com/android/server/devicepolicy/NetworkLoggingHandler.java index 957d4c54c2f9..baa4c13f5190 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/NetworkLoggingHandler.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/NetworkLoggingHandler.java @@ -17,16 +17,11 @@ package com.android.server.devicepolicy; import android.app.admin.DeviceAdminReceiver; -import android.app.admin.ConnectEvent; -import android.app.admin.DnsEvent; import android.app.admin.NetworkEvent; -import android.net.IIpConnectivityMetrics; -import android.net.INetdEventCallback; import android.os.Bundle; import android.os.Handler; import android.os.Looper; import android.os.Message; -import android.os.RemoteException; import android.util.Log; import com.android.internal.annotations.GuardedBy; @@ -44,10 +39,9 @@ final class NetworkLoggingHandler extends Handler { static final String NETWORK_EVENT_KEY = "network_event"; - // est. ~128kB of memory usage per full batch TODO(mkarpinski): fine tune based on testing data // If this value changes, update DevicePolicyManager#retrieveNetworkLogs() javadoc private static final int MAX_EVENTS_PER_BATCH = 1200; - static final long BATCH_FINALIZATION_TIMEOUT_MS = TimeUnit.MINUTES.toMillis(90); + private static final long BATCH_FINALIZATION_TIMEOUT_MS = TimeUnit.MINUTES.toMillis(90); static final int LOG_NETWORK_EVENT_MSG = 1; static final int FINALIZE_BATCH_MSG = 2; @@ -78,31 +72,32 @@ final class NetworkLoggingHandler extends Handler { if (networkEvent != null) { mNetworkEvents.add(networkEvent); if (mNetworkEvents.size() >= MAX_EVENTS_PER_BATCH) { - finalizeBatchAndNotifyDeviceOwner(); + finalizeBatchAndNotifyDeviceOwnerIfNotEmpty(); } } break; } case FINALIZE_BATCH_MSG: { - finalizeBatchAndNotifyDeviceOwner(); + finalizeBatchAndNotifyDeviceOwnerIfNotEmpty(); break; } } } - void scheduleBatchFinalization(long delay) { + void scheduleBatchFinalization() { removeMessages(FINALIZE_BATCH_MSG); - sendMessageDelayed(obtainMessage(FINALIZE_BATCH_MSG), delay); + sendMessageDelayed(obtainMessage(FINALIZE_BATCH_MSG), BATCH_FINALIZATION_TIMEOUT_MS); + Log.d(TAG, "Scheduled new batch finalization " + BATCH_FINALIZATION_TIMEOUT_MS + + "ms from now."); } - private synchronized void finalizeBatchAndNotifyDeviceOwner() { - mFullBatch = mNetworkEvents; - // start a new batch from scratch - mNetworkEvents = new ArrayList(); - scheduleBatchFinalization(BATCH_FINALIZATION_TIMEOUT_MS); - // notify DO that there's a new non-empty batch waiting - if (mFullBatch.size() > 0) { + private synchronized void finalizeBatchAndNotifyDeviceOwnerIfNotEmpty() { + if (mNetworkEvents.size() > 0) { + // finalize the batch and start a new one from scratch + mFullBatch = mNetworkEvents; mCurrentFullBatchToken++; + mNetworkEvents = new ArrayList(); + // notify DO that there's a new non-empty batch waiting Bundle extras = new Bundle(); extras.putLong(DeviceAdminReceiver.EXTRA_NETWORK_LOGS_TOKEN, mCurrentFullBatchToken); extras.putInt(DeviceAdminReceiver.EXTRA_NETWORK_LOGS_COUNT, mFullBatch.size()); @@ -110,8 +105,14 @@ final class NetworkLoggingHandler extends Handler { + mCurrentFullBatchToken); mDpm.sendDeviceOwnerCommand(DeviceAdminReceiver.ACTION_NETWORK_LOGS_AVAILABLE, extras); } else { - mFullBatch = null; + // don't notify the DO, since there are no events; DPC can still retrieve + // the last full batch + Log.d(TAG, "Was about to finalize the batch, but there were no events to send to" + + " the DPC, the batchToken of last available batch: " + + mCurrentFullBatchToken); } + // regardless of whether the batch was non-empty schedule a new finalization after timeout + scheduleBatchFinalization(); } synchronized List retrieveFullLogBatch(long batchToken) { -- 2.11.0